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

Speedup Local-Non-Local Context's data operations #5125

Closed
wants to merge 1 commit into from

Conversation

franz1981
Copy link
Contributor

@franz1981 franz1981 commented Feb 19, 2024

This is addressing 3 performance problems:

  • local/non-local context data's operations shouldn't use costy synchronized on existing maps (now that biased locking is gone)
  • local/non-local context data's lookup/remove doesn't really require to create any map, if not already present
  • local-non-local duplicated ctx data operations are not piggybacking on delegated ones, forcing using default subptimal implement ones, instead

This PR is pushing improvement furthers, but leveraging the facts that:

  • default's ConcurentHasMap constructor doesn't initialize the internal table, making very cheap to allocate one
  • most of the time there's no concurrent data allocations, which means we can save using synchronized and just use compareAndSet saving another strong memory barrier (in x86, but likely the same in ARM), speeding up common use cases

@vietj
Copy link
Member

vietj commented Feb 19, 2024

before doing anything here, we should first validate and finish this PR #5048 which has an impact on the overall design, since I would like to back port this PR to 4.x

@franz1981
Copy link
Contributor Author

franz1981 commented Feb 19, 2024

Let's wait till you sync with @cescoffier about the new API and the time to backport it: this one is pretty small and contained, so, depending for how long we will keep on using the current API, could be a waste or a good idea to bring it in.

@franz1981
Copy link
Contributor Author

@vietj vietj closed this Mar 1, 2024
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