RFR: 8340103: Add internal set_flag function to VMATree [v11]

Johan Sjölen jsjolen at openjdk.org
Tue Nov 5 08:19:31 UTC 2024


On Mon, 4 Nov 2024 14:53:51 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix incorrect merge
>
> src/hotspot/share/nmt/vmatree.hpp line 151:
> 
>> 149:   IntervalState& out_state(TreapNode* node) {
>> 150:     return node->val().out;
>> 151:   }
> 
> Why are these instance methods? Also, TreapNode* should be const. But maybe we can do things even better.
> 
> In cases like these I sometimes just use an ephemeral utility class. E.g.
> 
> 
> class VMATreeNode {
> const TreapNode* const _n;
> public:
> VMATreeNode(const TreapNode* node) .. assign _n
> MemTag mem_tag_in() const { return _n->val().in.mem_tag();  } 
> MemTag mem_tag_out() const { return _n->val().out.mem_tag();  } 
> ... all accessors in accessible form to save writing out the many dereferences ..
> };
> 
> 
> and then just use it as I go like this:
> 
> 
> TreapNode* node = ...
> if (VMATreeNode(node).mem_tag_in() == blabla )....
> 
> Bonus for a short name for VMATreeNode, because the code that really does anything then becomes a lot more readable.

They should be static and const, yes.

Re: ephemeral utility class. I do prefer using functions as opposed to methods, but it'd be nice to wrap them in a namespace or class that can be shortened and used outside of `VMATree`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20994#discussion_r1828910660


More information about the hotspot-runtime-dev mailing list