Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash enabled check to further improve performance #353

Merged
merged 1 commit into from
Jan 27, 2020

Conversation

ecktom
Copy link
Contributor

@ecktom ecktom commented Jan 26, 2020

Related issue

#346

Proposed changes

Just found one more thing to cache ;) Also thought about combining it with PipelineConfig but thought this one is clearer to understand and also with less changes

Checklist

  • I have read the contributing guidelines
  • I have read the security policy
  • I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security
    vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation within the code base (if appropriate)
  • I have documented my changes in the developer guide (if appropriate)

Further comments

Brings it down from 2 - 3ms to 0.1 - 0.3ms

Before:
profile002

After:
profile001

@ecktom ecktom requested a review from aeneasr January 26, 2020 19:31
@ecktom ecktom force-pushed the feat/hash_enabled branch from 4ad563a to c29ff2d Compare January 27, 2020 07:14
@aeneasr
Copy link
Member

aeneasr commented Jan 27, 2020

This is pretty insane! I would not have thought that viper has such a slow retrieval process! I think we should probably introduce caching to viper itself because this really isn't acceptable performance-wise!

Thank you for this change :)

@aeneasr aeneasr merged commit 19099cb into ory:master Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants