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

To obtain sessionId, sessions map is sought for value #208

Closed
dimaa6 opened this issue Sep 14, 2022 · 2 comments · Fixed by #209
Closed

To obtain sessionId, sessions map is sought for value #208

dimaa6 opened this issue Sep 14, 2022 · 2 comments · Fixed by #209

Comments

@dimaa6
Copy link

dimaa6 commented Sep 14, 2022

if (!sessions.containsValue(session)) {

This happens each time request is received. Server may be handling hundreds of thousands of connections, with hundreds of requests coming in every second. Synthetic test with map having 100,000 UUID keys pointing to String values and 1000 containsValue calls took 4 seconds. Test with the same map and 1000 containsKey calls took only 0.1ms. This shows that containsValue method does not utilize Map's ability to quickly find an object, instead, it iterates through all the values in regular scanning loop. If we really need to check if session is still present, we need to have Set of sessions in addition to the Map, and handle them together in atomic (synchronized) way - when adding a session, add it to both Map and Set, same when removing. Then Set will be used to quickly check if the session is still active.

@TVolden
Copy link
Member

TVolden commented Sep 14, 2022

Hi @dimaa6,

Thank you for reaching out. I'm happy to hear that business is good.

You are right of cause, it's a waste of resources to use containsValue. Especially if the UUID is already stored in the session via the getSessionId method.

Can I get you to create a PR for the change?

Sincerely
Thomas Volden

@dimaa6
Copy link
Author

dimaa6 commented Sep 14, 2022

Sure, will do tomorrow.

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 a pull request may close this issue.

2 participants