RFR: 8307533: Use atomic bitset functions for metadata flags

Kim Barrett kbarrett at openjdk.org
Sat May 6 05:42:18 UTC 2023


On Fri, 5 May 2023 19:58:49 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> Replace the bit set copies from metadata to use the Atomic functions.
> Tested with tier1-4.

Changes requested by kbarrett (Reviewer).

src/hotspot/share/oops/fieldInfo.inline.hpp line 159:

> 157: 
> 158: inline void FieldStatus::atomic_clear_bits(u1& flags, u1 mask) {
> 159:   u1 val = (~mask);

Why introduce a new variable?

src/hotspot/share/oops/fieldInfo.inline.hpp line 160:

> 158: inline void FieldStatus::atomic_clear_bits(u1& flags, u1 mask) {
> 159:   u1 val = (~mask);
> 160:   Atomic::fetch_then_and(&flags, val);

u1 is not a supported type for Atomic bitops.  This only happens to work right now because all
platforms are currently using a cmpxchg-based implementation and aren't enforcing the documented
limitation of only providing support for size of an int or size of a pointer (if different).

src/hotspot/share/oops/instanceKlassFlags.hpp line 127:

> 125: 
> 126:   void atomic_set_bits(u1 bits)   { Atomic::fetch_then_or(&_status, bits); }
> 127:   void atomic_clear_bits(u1 bits) { u1 val = (~bits); Atomic::fetch_then_and(&_status, val); }

Again here, u1 is not a supported type for Atomic bitops.

src/hotspot/share/oops/instanceKlassFlags.hpp line 127:

> 125: 
> 126:   void atomic_set_bits(u1 bits)   { Atomic::fetch_then_or(&_status, bits); }
> 127:   void atomic_clear_bits(u1 bits) { u1 val = (~bits); Atomic::fetch_then_and(&_status, val); }

Why introduce a new variable?

src/hotspot/share/oops/methodFlags.hpp line 91:

> 89:   int as_int() const { return _status; }
> 90:   void atomic_set_bits(u4 bits)   { Atomic::fetch_then_or(&_status, bits); }
> 91:   void atomic_clear_bits(u4 bits) { u4 val = (~bits); Atomic::fetch_then_and(&_status, val); }

Why introduce a new variable (and why the extra parens).  Just

Atomic::fetch_then_and(&_status, ~bits);

-------------

PR Review: https://git.openjdk.org/jdk/pull/13843#pullrequestreview-1415699630
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641849
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641272
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641459
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641809
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641760


More information about the hotspot-dev mailing list