http://gwt-code-reviews.appspot.com/870801/diff/1/3 File dev/core/src/com/google/gwt/core/ext/typeinfo/JAbstractMethod.java (right):
http://gwt-code-reviews.appspot.com/870801/diff/1/3#newcode245 dev/core/src/com/google/gwt/core/ext/typeinfo/JAbstractMethod.java:245: return StringInterner.get().intern(realParameterName); On 2010/09/15 19:26:44, scottb wrote:
Your CL looks good, huge improvement, but I should mention this whole
param name
thing is a huge mess.
I can (and probably should) break this into an if statement. Though i'm not sure if you're complaining about that or the larger functionality that this is part of. http://gwt-code-reviews.appspot.com/870801/diff/1/9 File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java (right): http://gwt-code-reviews.appspot.com/870801/diff/1/9#newcode91 dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:91: unresolvedSimple.add(interner.intern(String.valueOf(simpleRef))); On 2010/09/15 19:26:44, scottb wrote:
Totally side band.... but if you have any brilliant ideas about how to
go from a
char[] -> interned String -- without having to create the intermediary
string --
I'm all ears.
nothing springs to mind at the moment. http://gwt-code-reviews.appspot.com/870801/diff/1/22 File dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java (right): http://gwt-code-reviews.appspot.com/870801/diff/1/22#newcode165 dev/core/src/com/google/gwt/dev/resource/impl/ResourceOracleImpl.java:165: f.getName().toLowerCase(Locale.ENGLISH)); On 2010/09/15 19:26:44, scottb wrote:
Not useful, the String is only used locally.
whoops. good catch. http://gwt-code-reviews.appspot.com/870801/diff/1/23 File dev/core/src/com/google/gwt/dev/resource/impl/ZipFileResource.java (right): http://gwt-code-reviews.appspot.com/870801/diff/1/23#newcode53 dev/core/src/com/google/gwt/dev/resource/impl/ZipFileResource.java:53: "jar:" + classPathEntry.getLocation() + "!/" + path); On 2010/09/15 19:26:44, scottb wrote:
This feels strange, since it's transient here. What's the advantage
to
interning? IE: who holds a reference to the result?
This turned out to be a bit subtle, but you're right that it doesn't need to be interned. I'll remove it. the gory details: I recently changed this line (it used to check instanceof jarfile which was broken) and knew that the JdtCompiler eventually called into it. However, it doesn't hold a ref, it actually immediately converts it to a character array see: JdtCompiler$Adapter.getFileName() http://gwt-code-reviews.appspot.com/870801/diff/1/24 File dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java (right): http://gwt-code-reviews.appspot.com/870801/diff/1/24#newcode187 dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java:187: String mangledName = StringInterner.get().intern(sb.toString()); On 2010/09/15 19:26:44, scottb wrote:
Are both this change and the other one needed? Seems like if you do
this one,
the only other place you'd need to intern is the call to
field.getName(). i'm not sure if I follow. I interned here because its callers (see findMostDerivedMembers and addMember) all store references to the result, so I only had to do the intern in one spot. this is a different string than the name that gets interned in addMemberIfUnique above. http://gwt-code-reviews.appspot.com/870801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
