RFR: 8243326: Cleanup use of volatile in taskqueue code

[Thomas Schatzl]

Hi,

On 22.04.20 13:04, Kim Barrett wrote:
>> 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.

CamelCase is afaik the current way for new code, but feel free to leave 
as is.
I agree with the concern about too many extra changes, so feel free to 
skip or defer.

> 
>> - 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?

I'd be good with UnderflowSize, but an assert_not_underflow(size_t size) 
method would be fine too as most (all?) of these extra uses are asserts.

> 
>> 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.

Okay, feel free to defer to that.

> 
> 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.

Okay.

Thanks,
   Thomas