Thanks Kim, Roman, Dan and Coleen for reviews and feedback.

I rebased the patch, fixed more alignments, renamed the bug, and rerun the test through tier1-3.

https://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.03.delta/
https://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.03/

Could I get reviews for this version? I'd also like to ask others to at least partially look at this:

1) Platform maintainers probably want to run this patch through their build system.
2) SA maintainers (CC:ed serviceability-dev)
3) JVMCI maintainers

Thanks,
StefanK

On 2019-08-14 11:11, Roman Kennke wrote:

Am 14.08.19 um 01:26 schrieb Kim Barrett:
On Aug 12, 2019, at 12:19 PM, Stefan Karlsson <stefan.karls...@oracle.com> 
wrote:

Hi Roman,

Kim helped me figuring out how to get past the volatile issues I had with the 
class markWord { uintptr_t value; ... } version. So, I've created a version 
with that:

https://cr.openjdk.java.net/~stefank/8229258/webrev.valueMarkWord.01/

I can go with either approach, so let me now what you all think.
I've finally had time to look at the first proposed change.

Comparing the first approach (an AllStatic MarkWord class and markWord
typedef'd to uintptr_t) vs the second approach (markWord is a thin
class wrapping around uintptr_t), I prefer the second.

* I think the markWord class provides better type safety. It still
involves too many casts sprinkled over the code base, but I think it
also provides a better basis for further cast reduction and
prevention.

* I think having one markWord class for the data and behavior is
better / more natural than having a markWord typedef for the data and
a MarkWord AllStatic class for the behaviour.

* I like that the markWord class eliminates the markWord vs MarkWord
homonyms, which I think will be annoying.

* The markWord class is a trivially copyable class, allowing it to be
efficiently passed around by value, so no disadvantage there.

I haven't found anything that I think argues for the first over the
second. Other folks might have different priorities or taste. I think
either is better than the status quo.

I'm still reviewing webrev.valueMarkWord.02, but so far haven't found
anything that makes me want to suggest backing off from that direction.

Note that the bug summary doesn't describe the second approach.
+1 :-)

Roman


Reply via email to