-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Put session data in single field for speed #18913
Conversation
sweeeet 👍 |
$encryptedValue = $this->crypto->encrypt(json_encode($value), $this->passphrase); | ||
$this->session->set($key, $encryptedValue); | ||
$this->sessionValues[$key] = $value; | ||
$encryptedValue = $this->crypto->encrypt(json_encode($this->sessionValues), $this->passphrase); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also batch writes? maybe write in the destructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented
ddbc277
to
fd5c360
Compare
|
fd5c360
to
b085071
Compare
This prevents decrypting values multiple times.
b085071
to
0b91087
Compare
A new inspection was created. |
@icewind1991 @MorrisJobke Mind a review? THX. |
👍 looks good |
better 👍 |
Put session data in single field for speed
Regression: #19273 |
Nope. Not me to blame. 🙊 🙈 🙉 |
I didn't git blame you, only git bisected 😉 |
Hahaha 😄 😆 |
This will result in only one decryption and encryption call per request resulting in a better performance. Now the overhead is about 2ms for the encryption which is negligible. Especially since I found another bottleneck that stealing us
20ms50ms that I have fixed in #18914 🙊Ref #18482 (comment)
cc @karlitschek @icewind1991