RFR: 8274178: Occupancy value in logging and JFR event is inaccurate in G1IHOPControl

[Man Cao]

Do we want to rename the wording "current occupancy" and "occupancy" in
logging to something else? If we keep them as
G1CollectedHeap::heap()->used(), it is inconsistent with the other logging
message in G1Policy::need_to_start_conc_mark():

bool G1Policy::need_to_start_conc_mark(const char* source, size_t
alloc_word_size) {
  ...
  size_t cur_used_bytes = _g1h->non_young_capacity_bytes();
  ...
  if (marking_request_bytes > marking_initiating_used_threshold) {
    ...
    log_debug(gc, ergo, ihop)("%s occupancy: " SIZE_FORMAT "B allocation
request: " SIZE_FORMAT "B threshold: " SIZE_FORMAT "B (%1.2f) source: %s",
                              result ? "Request concurrent cycle initiation
(occupancy higher than threshold)" : "Do not request concurrent cycle
initiation (still doing mixed collections)",
                              cur_used_bytes, alloc_byte_size,
marking_initiating_used_threshold, (double)
marking_initiating_used_threshold / _g1h->capacity() * 100, source);
  }
}

The "occupancy" value in this message is already non_young_capacity_bytes().

Also, if we report both heap()->used() and non_young_capacity_bytes() in
log, which value should we use for "current_occupancy" in
G1NewTracer::report_basic_ihop_statistics() and
G1NewTracer::report_adaptive_ihop_statistics()?

-Man

On Thu, Oct 7, 2021 at 11:17 PM Thomas Schatzl <thomas.schatzl at oracle.com>
wrote:

> Hi,
>
> On 08.10.21 01:08, Jonathan Joo wrote:
> > Hi Thomas,
> >
> > Thanks for taking a look! I just wanted to clarify what you meant from
> your
> > message - for the field that I'm replacing ("occupancy"), are you saying
> > that we do indeed want to look at the total used heap space, even though
> > get_conc_mark_start_threshold() is depending on the non_young occupancy
> > bytes? In other words, the actual_target values being logged are related
> to
> > heap()->used(), while the get_conc_mark_start_threshold() values are
> > related to non_young_capacity_bytes()?
> >
> > If so, would an appropriate change be something like keeping the original
> > code, but adding a single new field in the debug message after
> "threshold"
> > with something like: "non-young usage (compared against threshold): " +
> > G1CollectedHeap::heap()->non_young_capacity_bytes()?
>
> Yes, that's what I suggested.
>
> Thanks,
>    Thomas
>