6633613: (str) StringCoding optimizations to avoid unnecessary array copies with Charset arg
iris clark
iris at sun.com
Tue Mar 4 02:02:43 UTC 2008
Hi, Martin.
> 6633613: (str) StringCoding optimizations to avoid unnecessary array
> copies with Charset arg
Not necessarily your problem, but it would be nice if your "Subject:"
line contained the word "review" or "review request" somewhere in it.
It would make this message easier to find. I suppose I should come up
with a recommendation, float it about, and get it written up in the
appropriate section of the Guide.
> with the patches below (a more ambitious patch will hopefully follow
> later):
Presumably I'll see that review later under one of the charset bugids?
> First, warning suppression:
I don't believe that StringCoding.java produces build warnings using
the default javac options. Should I assume that you're talking about
eliminating all build warnings produced using "-Xlint:all" for this
file only (not all of the String* files)?
> diff --git a/src/share/classes/java/lang/StringCoding.java
> b/src/share/classes/java/lang/StringCoding.java
These changes are fine.
> second, actual fix:
>
> diff --git a/src/share/classes/java/lang/StringCoding.java
> b/src/share/classes/java/lang/StringCoding.java
The code you modified was added _extremely_ late during jdk6
development to fix another bug. There's a regression test for that
bug. Hopefully you ran it? (Not that I expect it to fail... )
Speaking of regression tests, you didn't include one in these diffs
and I don't see an appropriate "noreg-*" keyword in the bug. Please
resolve this. See Step 6 ofthe Guide, "Change Planning and
Guidelines" [1] for a list of possible keywords.
The supplied fix is the minimal amount of change necessary to address
this bug (as indicated in your evaluation). Assuming that you still
intend to further modify code to improve performance in this area,
please remember to add the relevant bugids to the evaluation (or at
the very least, references to these bugs in the "See Also" section).
Finally, I know that we don't have an externally-visible repository
for webrevs yet, but we do need to keep a webrev of these changes as
it would aid review should we need to backport this fix.
Thanks,
iris
[1] http://openjdk.java.net/guide/changePlanning.html
More information about the core-libs-dev
mailing list