review request (L) - 6423256 GC stacks

[Scott Marlow]

John,

A few comments.  First, thank you for bringing this change out for review.

1.  If anyone on the list happens to have any performance jdk tests 
handy, it would be interesting to see before/after results of applying 
this change (so that any regressions can be addressed and improvements 
shared :-).  If there is no noticeable impact, that is worth sharing 
too.  I no longer have access to the torture test that I once had (a 
four node cluster that reproduced a production environment that 
experienced bug # 6423256).  If no one has performance tests to run, 
that shouldn't block applying this patch (would just be nice to have).

2.  I like the idea expressed in stack.hpp about possibly caching 
segments in thread local storage.  At least, that is the optimization 
that I assume is meant by saving the thread id.  Or are you just talking 
about using it for debugging purposes (for tracking which thread 
allocated each segment)?  I have worked on heap sub-allocators for other 
virtual machines but haven't learned about the Java native allocators 
yet.  I would think that having a per thread segment cache, could have 
some impact on the native memory pool (potentially causing fragmentation 
depending on how many threads are in use).  Although, the pressure on 
the native heap should be much lower than what growableArray must of 
done to the heap.  This isn't a serious concern, since its just a 
comment but thought it was worth raising.

3.  What is the meaning of "const size_t limit = size_t(-1) - (seg_size 
- 1)" in stack.inline.hpp (adjust_max_size)?  It looks like you are 
trying to determine the largest possible size but I wanted to make sure 
as I'm not familiar with how "size_t(-1)" works.  Could there be 
problems with that expression on some of the exotic machine 
architectures that Java is running on (e.g. 1s complement cpu running 
j2me).

Scott

On 09/21/2009 01:09 PM, John Coomes wrote:
> I'd appreciate reviews of
>
> 	6423256: GC stacks should use a better data structure
>
> 	http://cr.openjdk.java.net/~jcoomes/6423256-gc-stacks/
>
> See the comments at the top of the webrev and in file stack.hpp for
> some hints/details.
>
> -John
>
>
>