RFR: 8227054: ServiceThread needs to know about all OopStorage objects
Kim Barrett
kim.barrett at oracle.com
Mon Aug 12 03:26:16 UTC 2019
> On Aug 10, 2019, at 3:52 AM, Erik Osterlund <erik.osterlund at oracle.com> wrote:
>
> Hi Kim,
>
> Thank you for taking this for another spin. I like the new version a lot better. I think you convinced me on using the private enum being a better idea than my reinterpret_cast.
>
> I only have two questions left:
>
> 1) How would you feel about not using the macro for generating the getters? It looks like 1 short line for the getter vs 1 short line for the macro. In other words, not much typing is saved using the X macro here. But it will confuse IDEs. So I would have preferred the explicit getters.
>
> 2) Similar thought for the initialization of OopStorages where the name is generated from the macro. I thought the name was meant to be in more human readable form as we print them out in logs. So again, I would have loved explicit initialization, which is still just 1 trivial line per OopStorage.
>
> If you have strong opinions about keeping those macros, I will not oppose it. But maybe you agree that retaining that IDE awareness of the code and nice printouts in the logs is worth doing those one liners without X macros?
Just to be clear, there are no x-macros here, just a couple very
simple macros. Using x-macros would have reduced those blocks of
macro invocations to one-liners.
Can you describe the IDE confusion? I can think of lots of potential
issues, depending on how smart/not-so-smart a given IDE might be, but
I don't know what actual problems you might be worried about.
For me, the macros improve readability by emphasizing the parts that
are different (the interesting parts) and deemphasizing the parts that
are the same (the boring parts).
I looked at the logging of OopStorage processing; the use of the
stringized identifiers rather than bespoke strings looked fine to me.
Indeed, it has the small additional benefit for someone who is a
developer that the name exactly matches the associated getter. (Of
course, that doesn't matter to a non-developer log user.)
> Apart from those two things, I really liked the approach!
Thanks. Hopefully we’re approaching a conclusion on this change.
More information about the hotspot-dev
mailing list