review (M) for 6892658: C2 should optimize some stringbuilder patterns

[Tom Rodriguez]

On Nov 10, 2009, at 11:42 AM, Christian Thalinger wrote:

> On Mon, 2009-11-09 at 15:46 -0800, Tom Rodriguez wrote:
>> http://cr.openjdk.java.net/~never/6892658/
> 
> src/share/vm/memory/universe.hpp:
> 
> +   static oop          _the_MIN_VALUE_string;          // A cache of "" as a Java string
> 
> The min-value is missing in the comment.

Fixed.

> src/share/vm/opto/callGenerator.cpp:
> 
> void WarmCallInfo::make_hot() {
> 
> Why is this method now Unimplemented()?

make_hot is a non-working implementation of late inlining and long term I think all that code should be deleted and rebuilt if we intend to move more generally to a late inlining strategy.  I just went ahead and deleted that for now.  I could leave it behind if you like.

> 
> 
> src/share/vm/opto/callnode.cpp:
> 
> +   // The resproj may not exist because the result couuld be ignored
> +   // and the exception object may not be exist if an exception handler
> 
> Typo.

Fixed.

> 
> 
> src/share/vm/opto/compile.cpp:
> 
> +         // Separate projections were use for the exception path which
> 
> "were used"?

Fixed.

> 
> 
> src/share/vm/opto/phase.hpp:
> 
>      LIVE,                       // Dragon-book LIVE range problem
> +     StringOpts,
>      Interference_Graph,         // Building the IFG
> 
> A comment would be nice.

Fixed.

> src/share/vm/utilities/growableArray.hpp:
> 
> +   /* inserts the given element before the element at index i */                       
> 
> A C comment?

It started as a copy paste from a context that couldn't use C++ style comments and I never fixed it.  I've corrected it.

tom

> 
> I will look at src/share/vm/opto/stringopts.cpp tomorrow.
> 
> -- Christian
>