RFR: 8227054: ServiceThread needs to know about all OopStorage objects

[Kim Barrett]

> On Aug 5, 2019, at 3:33 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
> 
> Hi Kim,
> 
> On 2019-08-05 04:35, Kim Barrett wrote:
> 
> This conversation whether we should or should not care about the aliasing rules has been repeated many times. And the answer is always the same: we should not care about aliasing rules. Summary of why:
> 
> * HotSpot inherently can't comply to the aliasing rules, as those rules are only defined in terms of C++ accesses interacting with each other. Since we JIT code performing accesses on the same memory as the C++ code, the aliasing rules can inherently never be trusted, as our interactions cross language barriers.
> * Compilers used to compile HotSpot have all been told to ignore aliasing rules, and hence the behaviour of doing so is well defined. Because it has to be. If a compiler can't ignore aliasing, it can't compile HotSpot.
> * Even if we were not inherently tied to ignore these aliasing rules, a significantly large part of HotSpot relies on it, and we can't realistically get rid of that reliance to turn the flags off. At least not by taking huge risk that we think we caught all the places, which would be rather irresponsible.
> * Even if we could magically get rid of this reliance and somehow rewrite all code to conform with the aliasing rules, verify that we solved it all, and switch the compiler flags ignoring aliasing off, which would be a crazy amount of work, and be very risky, the benefits are seemingly unclear. They were introduced to allow compiler optimizations. I have not seen studies with numbers showing that it improves things. I have heard vague statements that it does, but not with numbers. I have found one study - a paper from Purdue where they had run with and without -fstrict-aliasing on a number of benchmarks, and observed no benefit. And most our time is spent in JIT:ed code, so I would expect even less if anything.
> * Given the above, HotSpot does not, can not, and never will care about aliasing. They doe not make sense in our context, and the rules simply don't apply. Therefore, we can and should ignore the aliasing rules. Caring about them "sometimes" makes no sense to me. That's why I say I don't mind violating the aliasing rules, because they simply don't apply to HotSpot.

Those arguments are the rationale we use for breaking the rules at
all.  But they aren't good reasons for breaking the rules casually.

reinterpret_cast (however spelled) takes a big hammer to the type
system.  It should only be used when there aren't other options.
Besides ignoring aliasing rules, it can have other knock-on effects
and generally makes review and later maintenance more difficult.

You feel haranged in Italian when you see macros.  When I see a
reinterpret_cast I hear furtive incantations in the language of
R'lyeh, and fear for my sanity!

>>>> And an iterator whose initial "current" is invalid?  You think
>>>> *macros* are confusing?
>>> 
>>> That’s Java style iterators I guess. You call next until you can’t. Before you called next the first time, there is no current element that has been handed out. I don’t mind changing the iterator style though if you think that is confusing. It is my preferred style for simple linear iteration.
>> In this case that idiom seems like it expends additional effort to
>> potentially hide bugs.
> 
> I don't know what you mean by that.

That form of iterator does the range check in the iterator to decide
whether to dereference or return NULL.  The caller must then check for
NULL, even though it's a postcondition that if the range check passed
then the value is never NULL (because it's a precondition that all the
OopStorage* slots have been filled in, else iteration could face early
termination unexpectedly, which would be a bug).  But we have no way
to communicate those pre/postconditions to the compiler, so the NULL
check must dutifully be done on the dereference case, even though it's
a complete waste and could hide a bug of using it too soon.


>> It also seems less convenient to use, though if one uses implicit
>> booleans (as the proposed code does), that disadvantage is nullified.
>> (I've had many occasions to want to use the declaration form of a
>> condition, but haven't because there were strong complaints when I
>> first tried doing so, and I've since seen it objected to when others
>> tried.  But I found some recently added, so maybe that's no longer
>> true?  Or maybe the previous complainers didn't notice.)
> 
> Yeah this style is indeed only nice when using the declaration form of a condition. If anyone thinks that should not be allowed for some reason, I don't have strong opinions about having a different iterator style.

As I said, I like the feature, but it *is* a violation of the
no-implicit-bools style guide rule, and there were definite opinions
expressed the one time I attempted to use it.