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