RFR: 8243326: Cleanup use of volatile in taskqueue code

Kim Barrett kim.barrett at oracle.com
Wed Apr 22 11:04:55 UTC 2020


> On Apr 22, 2020, at 6:06 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi,
> 
> On 22.04.20 10:49, Kim Barrett wrote:
>> Please review this change that eliminates most uses of the volatile
>> qualifier in the taskqueue code.  The remaining uses are in the
>> declarations of the queue's _bottom and _age members.  That's just our
>> current style of tagging "atomic" variables manipulated by multiple
>> threads.
> 
> Some minor nits:
> 
> - although I understand it is quite old code, maybe change MOD_N_MASK to CamelCase in some way as it is a static const now. I am okay to ignore this remark.

So far as I can tell, we don't have a consistent naming convention for
constants. It's easy to find examples for all of CamelCase, snake_case,
and UPPER_CASE.  I was also trying not to do too many extraneous changes.

> - pre-existing: static-assert that N is a power of 2 otherwise the masking won't work.
> 
> - pre-existing: N should also be smaller than what is representable by idx_t.

Done.  is_power_of_2 is not (yet) constexpr, so had to be manually inlined.

> - pre-existing: there are a few places/asserts where "N - 1" is used verbatim. Maybe exchange with that static above, however I'm not sure that unless it is named properly this is an improvement. Or have another constant for that. Or have a predicate/method for that always same-checking. Probably it's best to ignore this comment.

Something to do with underflow?  UNDERFLOW_SIZE, UnderflowSize?

> Looks good otherwise.

Thanks.

I have at least one more cleanup to go here anyway: JDK-8243325.  I
could add some more for the constant names.

I prototyped a substantial restructuring of the classes, but decided
not to mix that in with the other changes, if I pursued it at all.
The use of templates in the current code is pretty clumsy.  For
example, the MEMFLAGS parameter affects *everything*, even though very
little code actually depends on it.  This may lead to a significant
amount of code bloat.

I'm going to hold off on posting an updated webrev to see if there are
any other comments.




More information about the hotspot-gc-dev mailing list