On Sat, 9 Apr 2022 06:41:11 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:

>>> On Apr 9\, 2022\, at 2\:44 AM\, Thomas Stuefe \<stuefe at 
>>> openjdk\.java\.net> wrote\:
>>> 
>>> On Fri\, 8 Apr 2022 17\:34\:57 GMT\, Roman Kennke \<rkennke at 
>>> openjdk\.org> wrote\:
>>> 
>>>> Yes\, I get that\. But the fragments and fragment\-table are themselves 
>>>> inner classes that take a MEMFLAGS template\. I could \(perhaps\) either 
>>>> use a constexpr MEMFLAGS arg and pass this through\, or do at some point a 
>>>> switch like\:
>>>> 
>>>> \`\`\`
>>>> switch \(\_flags\) \{
>>>>  case mtServiceability\:
>>>>  \.\.\. new BitMapFragmentTable\<mtServiceability>\(\)\; break\;
>>>>  case mtServiceability\:
>>>>  \.\.\. new BitMapFragmentTable\<mtServiceability>\(\)\; break\;
>>>>  default\: ShouldNotReachHere\(\)\;
>>>> \}
>>>> \`\`\`
>>>> 
>>>> Which seems kinda\-ugly but would work \(I think\)\, and avoid making the 
>>>> outer class template\-ized\.
>>> 
>>> I see what you mean\. This MEMFLAGS template parameter is deeply interwoven 
>>> into everything\. I\'d just live with the current solution\. It uses 
>>> established pattern\, so at least nobody is surprised\.
>>> 
>>> I think the basic problem is that CHeapObj itself is a template class\. 
>>> Rethinking MEMFLAGS seems worthwhile for a future RFE\. As I wrote\, one 
>>> approach could be to make them a property of the current thread\, and 
>>> switchable and stackable via a Mark class\. That way\, everything allocated 
>>> within a given range of frames would count toward a given category\. No 
>>> need to decide on a fine\-granular basis\. No need for templates\. Maybe no 
>>> need even to have a MEMFLAGS argument for every allocation\.
>> 
>> While working on something else I ran into a similar problem and found a 
>> different
>> approach that seemed to work well\.  I\?m planning to explore it in the 
>> context of
>> CHeapObj\, but haven\?t gotten around to it yet\.  I should file an RFE in 
>> case someone
>> else is interested\.
>
>> Yes, I get that. But the fragments and fragment-table are themselves inner 
>> classes that take a MEMFLAGS template. I could (perhaps) either use a 
>> constexpr MEMFLAGS arg and pass this through, or do at some point a switch 
>> like:
>> 
>> ```
>> switch (_flags) {
>>   case mtServiceability:
>>   ... new BitMapFragmentTable<mtServiceability>(); break;
>>   case mtServiceability:
>>   ... new BitMapFragmentTable<mtServiceability>(); break;
>>   default: ShouldNotReachHere();
>> }
>> ```
>> 
>> Which seems kinda-ugly but would work (I think), and avoid making the outer 
>> class template-ized.
> 
> I see what you mean. This MEMFLAGS template parameter is deeply interwoven 
> into everything. I'd just live with the current solution. It uses established 
> pattern, so at least nobody is surprised.
> 
> I think the basic problem is that CHeapObj itself is a template class. 
> Rethinking MEMFLAGS seems worthwhile for a future RFE. As I wrote, one 
> approach could be to make them a property of the current thread, and 
> switchable and stackable via a Mark class. That way, everything allocated 
> within a given range of frames would count toward a given category. No need 
> to decide on a fine-granular basis. No need for templates. Maybe no need even 
> to have a MEMFLAGS argument for every allocation.

Have we reached a consensus that the current proposal is the way to go? If so, 
could you please mark the latest revision as Reviewed (again), @tstuefe and 
@coleenp (and whoever else feels like doing so)?

I also added some basic gtests for ObjectBitSet. Please note my remark on the 
NULL test case.

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

PR: https://git.openjdk.java.net/jdk/pull/7964

Reply via email to