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

Improve the smallIntegerValueCache added during InclusiveRange implementation #2871

Closed
Tracked by #2482
darkdrag00nv2 opened this issue Oct 14, 2023 · 1 comment
Closed
Tracked by #2482

Comments

@darkdrag00nv2
Copy link
Contributor

Issue to be solved

The cache is thread safe and we should measure if there is an advantage in having the cache vs the downside of having a lock.

Comment from @turbolent on #2523

Given that the cache is global, different interpreter runs, in tests or not, might concurrently hit the cache, so it needs to be safe. So far all other "state" is interpreter-instance specific and not global.

My gut feeling is that the overhead of making this thread-safe and lock contention might be worse than the benefits of caching/re-using existing values. We might want to check and optimize this if needed, Maybe best to do that in a follow-up PR, this one is already quite large.

Suggested Solution

No response

@darkdrag00nv2
Copy link
Contributor Author

Based on the benchmark results from #2936, we are okay with using the cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants