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

Core: remove ConcurrentHashMapV8 #7296

Closed
mikemccand opened this Issue Aug 15, 2014 · 5 comments

Comments

Projects
None yet
2 participants
@mikemccand
Copy link
Contributor

mikemccand commented Aug 15, 2014

In #6400 we pulled in a snapshot of CHM V8 to reduce RAM overhead, notable for the live version map.

But I think this is actually quite risky going forward .. the implementation in the OpenJDK has already changed quite a bit since Doug Lea's version, and it's risky if we don't pull in bug fixes.

I think we should just revert back to the JVM's implementation? Users can upgrade to Java 8 to reduce RAM usage ... I think this means we need to conditionalize the RAM usage logic in LiveVersionMap.

@mikemccand

This comment has been minimized.

Copy link
Contributor Author

mikemccand commented Aug 15, 2014

Also, I think separately (#6392) we can think about more RAM efficient data structures to hold live versions ... then we don't rely on CHM's RAM efficiency here.

@cfontes

This comment has been minimized.

Copy link
Contributor

cfontes commented Aug 18, 2014

How can I help?

This _looks_ easy enough for a newcomer like me.

@mikemccand

This comment has been minimized.

Copy link
Contributor Author

mikemccand commented Aug 18, 2014

Hi @cfontes I think it should be straightforward; it should amount to just reverting the commit from #6400? Do you have an iCLA on file?

@cfontes

This comment has been minimized.

Copy link
Contributor

cfontes commented Aug 19, 2014

Ok.

I do now.

@cfontes

This comment has been minimized.

Copy link
Contributor

cfontes commented Aug 22, 2014

Here it is #7392

mikemccand added a commit that referenced this issue Aug 26, 2014

Core: use Java's built-in ConcurrentHashMap
It's risky to have our own snapshot of Java 8's ConcurrentHashMap:
unless we keep the sources in sync over time (and OpenJDK's version
had already diverged), then we won't get bug/performance fixes.  Users
can choose to upgrade to Java 8 to see the improvements of CHM.

Closes #7392

Closes #7296

mikemccand added a commit that referenced this issue Sep 8, 2014

Core: use Java's built-in ConcurrentHashMap
It's risky to have our own snapshot of Java 8's ConcurrentHashMap:
unless we keep the sources in sync over time (and OpenJDK's version
had already diverged), then we won't get bug/performance fixes.  Users
can choose to upgrade to Java 8 to see the improvements of CHM.

Closes #7392

Closes #7296
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.