RFR: 8214278: Cleanup process_completed_threshold and related state

[Thomas Schatzl]

Hi Kim,

On Sun, 2018-11-25 at 20:44 -0500, Kim Barrett wrote:
> This change addresses two CRs:
> 8214278: Cleanup process_completed_threshold and related state  
> 8156696: Simplify PtrQueueSet initialization 
> 
> This change is based on the change for 8214202:
> DirtyCardQueueSet::get_completed_buffer should not clear
> _process_completed, which is already out for review.
> 
> Please review this change to some of the data members in PtrQueueSet,
> to simplify their semantics and usage.  The affected data members are
> 
> _process_completed_threshold
> _max_completed_queue 
> _completed_queue_padding
> 
> The types of all of them are now size_t (some were int).  There is no
> longer any special treatment of zero (and negative is no longer
> possible).  The only special treatment now required is the
> possibility of overflow when adding max completed and padding.
> 
> The members are now initialized to extreme values (either 0 or max,
> as appropriate), with PtrQueueSet::initialize no longer taking any
> associated arguments.  Instead, through proper choice of the initial
> values, some require no further initialization, and the rest are
> updated via the associated setters.  (This addresses JDK-8156696.)
> 
> Added missing const qualifiers for associated reader functions.
> 
> Finally, the following renamings were made, for improved naming
> consistency:
> 
> process_completed_threshold => process_completed_buffers_threshold
> max_completed_queue => max_completed_buffers
> completed_queue_padding => completed_buffers_padding
> 
> (There are still some naming inconsistencies around "completed
> buffers", which I'm planning to address in future RFEs.)
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8214278
> https://bugs.openjdk.java.net/browse/JDK-8156696
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8214278/open.00/
> 
> Testing:
> mach5 tier1-5
> 

Thanks for doing this cleanup.

Some minor nits:

- in DirtyCardQueueSet::concatenate_logs() a SizeTFlagSetting could be
used to do the temporarily-change-value dance. (I know, the class is
very specifically named).

- not sure if static consts like
_process_completed_buffers_threshold_never should be named like a
member. I think at some point we agreed on (but did not write down) to
use CamelCase for those.

Just mentioning: another option than doing the overflow check in
PtrQueueSet::process_or_enqueue_complete_buffer could be limiting
_completed_buffers_padding to something like INT_MAX as we already do
for _max_completed_buffers.
But I think the existing check is less noise than changing the padding
code.

Thanks,
  Thomas