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