Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-14 Thread Thomas Stuefe
On Mon, 13 May 2024 10:17:57 GMT, Stefan Karlsson wrote: >> Thomas Stuefe has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update mallocLimit.hpp > > Changes requested by stefank (Reviewer). @stefank New version, hopefully addressed all

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-14 Thread Thomas Stuefe
On Mon, 13 May 2024 15:48:43 GMT, Stefan Karlsson wrote: >> I tend to agree with that. My earlier question still stands: is there a >> better place to put it? Right now the "enforced with code" in a stand-alone >> file doesn't tell me "why" this rule is important. > > If you want to keep the

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-14 Thread Thomas Stuefe
On Mon, 13 May 2024 14:47:18 GMT, Stefan Karlsson wrote: >> I don't feel like starting that particular bike shedding discussion :) But >> sure, sometime in the future we should do this. Here, I want it to be a >> simple renaming change. > > Right. That's why I prefixed this with "Open-ended

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Kim Barrett
On Mon, 13 May 2024 04:55:24 GMT, Thomas Stuefe wrote: >> MEMFLAGS, as well as its enum constants, should live in its own include. >> >> The constants are used throughout the code base, often without needing the >> allocation APIs exposed through allocation.hpp. >> >> The MEMFLAGS enum def

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Stefan Karlsson
On Mon, 13 May 2024 15:36:13 GMT, Stefan Karlsson wrote: >> To quote @robehn - Why write a comment for a rule if you can enforce it with >> code instead... > > I tend to agree with that. My earlier question still stands: is there a > better place to put it? Right now the "enforced with code"

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Stefan Karlsson
On Mon, 13 May 2024 15:26:18 GMT, Daniel D. Daugherty wrote: >> Could you instead put the static_assert near the code that will break? Right >> now it looks obscure and weird to have this check when it is obviously >> correct as long as no one changes the definition. Would it be enough to >>

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Daniel D . Daugherty
On Mon, 13 May 2024 14:44:05 GMT, Stefan Karlsson wrote: >> I rather have this explicit check. If MEMFLAGS>1byte, things break, and I >> would like to make that explicit. >> >> That said, I can move this static assert to the header. I just wanted to >> avoid including debug.hpp. My original

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Stefan Karlsson
On Mon, 13 May 2024 14:31:22 GMT, Thomas Stuefe wrote: >> src/hotspot/share/nmt/memflags.cpp line 31: >> >>> 29: >>> 30: // Extra insurance that MEMFLAGS truly has the same size as uint8_t. >>> 31: STATIC_ASSERT(sizeof(MEMFLAGS) == sizeof(uint8_t)); >> >> I think you can remove this entire

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Thomas Stuefe
On Mon, 13 May 2024 10:13:50 GMT, Stefan Karlsson wrote: >> Thomas Stuefe has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update mallocLimit.hpp > > src/hotspot/share/nmt/memflags.cpp line 31: > >> 29: >> 30: // Extra insurance that

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Thomas Stuefe
On Mon, 13 May 2024 10:16:36 GMT, Stefan Karlsson wrote: >> Thomas Stuefe has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update mallocLimit.hpp > > src/hotspot/share/nmt/memflags.hpp line 30: > >> 28: #include

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Stefan Karlsson
On Mon, 13 May 2024 04:55:24 GMT, Thomas Stuefe wrote: >> MEMFLAGS, as well as its enum constants, should live in its own include. >> >> The constants are used throughout the code base, often without needing the >> allocation APIs exposed through allocation.hpp. >> >> The MEMFLAGS enum def

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-13 Thread Johan Sjölen
On Mon, 13 May 2024 04:55:24 GMT, Thomas Stuefe wrote: >> MEMFLAGS, as well as its enum constants, should live in its own include. >> >> The constants are used throughout the code base, often without needing the >> allocation APIs exposed through allocation.hpp. >> >> The MEMFLAGS enum def

Re: RFR: 8332042: Move MEMFLAGS to its own include file [v2]

2024-05-12 Thread Thomas Stuefe
> MEMFLAGS, as well as its enum constants, should live in its own include. > > The constants are used throughout the code base, often without needing the > allocation APIs exposed through allocation.hpp. > > The MEMFLAGS enum def is often needed within NMT itself, again often without >