CRR (re-opened / M): 7075646: G1: fix inconsistencies in the monitoring data
tony.printezis at oracle.com
Fri Sep 23 08:19:58 PDT 2011
On 9/22/2011 4:57 PM, Bengt Rutisson wrote:
> Looks good. Thanks for addressing my comments so quickly. Ship it! :-)
And thanks for all the helpful feedback!
>> Due to popular demand :-) I changed the #define for the undefined max
>> for the G1 memory pools to a static const.
> I like it! Thanks!
> Finally, a comment regarding the almost empty super classes
> G1MemoryPoolSuper and G1GenerationCounters. I see your point about
> collecting the instance variable for the G1MonitoringSupport instance
> in a super class, but I don't agree that having an extra super class
> "does not cost us anything". It might not cost us any performance, but
> it does cost us in code complexity and speed of figuring call
> hierarchies out.
Normally, yes. And it is true that some of our class hierarchies are way
too convoluted and actually counter-productive. But, in this case, the
extra overhead from that is minimal! Oh, and now there's of course a
second field in G1MemoryPoolSuper. ;-)
> Clearly there is no right or wrong here. Just a matter of balancing
> different code quality values against each other. So, I am fine with
> leaving these extra super classes in.
>> On 09/21/2011 06:34 PM, Tony Printezis wrote:
>>> (resending to the correct list this time....)
>>> Hi all,
>>> I'm re-opening this change for code review:
>>> Here's a quick recap of the changes:
>>> a) The jstat tool assumes that none of the spaces have 0 capacity.
>>> In G1 the assumption does not always hold (i.e., where no survivor
>>> regions are allocated, the survivor space has 0 capacity). This
>>> causes jstat to display an unexpected character (?) instead of the
>>> correct occupancy percentage of a space. To get round this problem I
>>> now artificially pad all capacities, when reported through jstat, to
>>> ensure that no capacity is 0.
>>> b) The jstat counters for the young / old gen capacity were not
>>> updated. Those counters are now updated correctly.
>>> Unfortunately, I had to re-architect the way G1 calculates the
>>> various sizes reported by the monitoring frameworks to ensure that
>>> we do minimal work when a new eden region is allocated (e.g., update
>>> exactly the eden occupancy counter, not having to update all the
>>> sizes and counters). The new version calculates all the sizes
>>> synchronously in well-defined places (all sizes: end of GC, end of
>>> hum object allocation, eden occupancy: after an eden region
>>> allocation) and stores them in fields. The consumers of the data
>>> only read the fields when they need the values, i.e., they do not
>>> need to recalculate anything.
>>> I also tidied up the GenerationCounters class (there are now two
>>> clear constructors, one for uses with a VirtualSpace, one without)
>>> and the G1 memory pool classes (we had several static wrapper
>>> methods that calculated some of the sizes, but these are not needed
>>> any more as the calculations are now done elsewhere).
More information about the hotspot-gc-dev