RFR (M): 8160897: Concurrent mark mark stack memory allocation leaks memory
Thomas Schatzl
thomas.schatzl at oracle.com
Wed Jul 13 17:48:21 UTC 2016
Hi Jon,
thanks for your quick review.
On Wed, 2016-07-13 at 09:26 -0700, Jon Masamitsu wrote:
> http://cr.openjdk.java.net/~tschatzl/8160897/webrev/src/share/vm/gc/g
> 1/g1ConcurrentMark.hpp.frames.html
>
> 175 // Pushes the first "n" elements of "ptr_arr" on the stack.
> 176 void par_push_arr(oop* buffer, size_t n);
>
> Comment should refer to "buffer" not "ptr_arr"?
>
> 177
> 178 // Moves up to max elements from the stack into the given
> buffer. Returns
> 179 // the number of elements pushed, and false if the array has
> been empty.
> 180 bool par_pop_arr(oop* buffer, size_t max, size_t* n);
>
>
> Do you want to comment on return value?
Done.
>
> What motivated this increase to 1024?
>
> 673 // How many entries will be transferred between global stack
> and
> 674 // local queues at once.
> 675 global_stack_transfer_size = 1024
- the local mark stack sizes (128k entries)
- the global mark stack size (I have been configuring some setups with
1M entries)
- the push and pop operations take a global lock
I.e. to significantly decrease the overhead.
Note that the follow-up CR to remove the use of the global lock will
remove that constant from there (and practically have the same
elsewhere).
However, that follow-up CR is an enhancement, so subject to an
exemption request.
>
> http://cr.openjdk.java.net/~tschatzl/8160897/webrev/src/share/vm/gc/g
> 1/g1ConcurrentMark.cpp.frames.html
>
> 143 set_empty();
>
> Isn't this taken care of by the constructors?
>
>
> 139 _index(0),
> 141 _overflow(false),
I removed the uses in the initialization list. I prefer to reuse
existing code.
>
>
> Does resize() try to reserve new space if the new_capacity is the
> same
> as the current capacity?
> 146 bool G1CMMarkStack::resize(size_t new_capacity) {
Yes. However, resize is private, and will never be called with the same
size.
> Have you ever cursed the fact that the may data structure (such as
> this) double capacity instead of growing more gradually? Time
> consider
> something different? Ignore this if it is not the time.
> 198 // Double capacity if possible
> 199 size_t new_capacity = MIN2(old_capacity * 2,
> MarkStackSizeMax);
There may be some gain to be had there, by using a slower growing
exponential function.
I will create an enhancement request to look into this.
>
> That's all.
Thanks a lot.
New webrev at
http://cr.openjdk.java.net/~tschatzl/8160897/webrev.1/ (full)
http://cr.openjdk.java.net/~tschatzl/8160897/webrev.0_to_1/ (diff)
Thanks,
Thomas
More information about the hotspot-gc-dev
mailing list