You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
It is not recommended to use String.intern() at all. Guava contains a better replacement in it's Interner class.
Gradle build tool has been using Guava's interner for years (since it was introduced in 2015 with gradle/gradle@799e03e6) and it has been a successful solution in reducing String instance duplication without the problems that String.intern() has.
Modifications
Replace String.intern() with the singleton instance that uses Guava Interner
might cause memory leaks outside of DoS attacks since the instances will never be collected by GC
Can you provide evidence of this statement? I'm surprised if it's the case it should be a JDK bug. And the original PR introduced these lines #952 used it to reduce String objects.
might cause memory leaks outside of DoS attacks since the instances will never be collected by GC
Can you provide evidence of this statement? I'm surprised if it's the case it should be a JDK bug. And the original PR introduced these lines #952 used it to reduce String objects.
@tisonkun String intern seemed to have improved in JDK 8 and my knowledge about that was outdated. I was wrong about "never be collected by GC". Interned String instances will get collected as the last resort, in Full GC.
I'll remove the DoS argumentation from the PR description. However, as I mentioned, Guava's Interner is a very well optimized solution for interning objects. It has been used in Gradle since 2015 and it's very performant.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
doc-not-neededYour PR changes do not impact docsready-to-testtype/cleanupCode or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
It is not recommended to use
String.intern()at all. Guava contains a better replacement in it's Interner class.Gradle build tool has been using Guava's interner for years (since it was introduced in 2015 with gradle/gradle@799e03e6) and it has been a successful solution in reducing String instance duplication without the problems that
String.intern()has.Modifications
Replace
String.intern()with the singleton instance that uses Guava InternerDocumentation
docdoc-requireddoc-not-neededdoc-complete