We need help to migrate from bucket 1 to 2; and, the == problem

[Dan Heidinga]

On Thu, May 26, 2022 at 1:34 PM Kevin Bourrillion <kevinb at google.com> wrote:
>
> I'd like to bump this thread, as it seems to me to be the biggest obstacle to bucket 2 being able to deliver value.
>
> * A warning not just on synchronization, but on *any* identity-dependence.

This will have high costs in the regular performance model as it will
require additional checks in the implementation of the if_acmp
bytecode (which today is basically a pointer comparison) and in
Object::hashcode(). I'm not convinced the community would welcome a
performance regression here to gain the warning.

> * Not special for Integer etc.; it all needs to work through a general facility that anyone can use.
>     * We don't need the constructor warnings, though.
> * The annotation should evoke the idea of "this class is becoming a bucket 2 class".
>     * It would be vestigial once the class *is* bucket-2.
>     * I would lean against enshrining the "value-based" terminology even further (we can get into this if necessary).
> * I think we need an explicit way to clearly and *intentionally* depend on identity. This code would *prefer to break* if the objects in use became bucket-2. e.g.:
>     * o1.identity() == o2.identity() // I like this
>     * System.identity(o1) == System.identity(o2) // this too
>     * System.identityEquals(o1, o2)
>     * o1 === o2

Are these marker methods?  What would they return?  Does this
implementation meet the requirements of what you're envisioning here?

Object identity(Object o) {
  if (o.isValue()) { throw new IAE(); }
  return o;
}

--Dan

>
> Thoughts?
>
>
> On Tue, Apr 26, 2022 at 3:09 PM Kevin Bourrillion <kevinb at google.com> wrote:
>>
>> Above, when I said the proposed `==` behavior is "not a behavior that anyone ever *actually wants* -- unless they just happen to have no fields of reference types at all", I did leave out some other cases. Like when your only field types (recursing down fields of value types) that are reference types are types that don't override `equals()` (e.g. `Function`). In a way this sort of furthers my argument that the boundary between when `==` is safely an `equals` synonym and when it isn't is going to be difficult to perceive. Yet, since people hunger for `==` to really mean `equals`, they are highly overwhelmingly likely to do it as much as possible whenever they are convinced it looks safe. And then one addition of a string field in some leaf-level type can break a whole lot of code.
>>
>>
>> On Tue, Apr 26, 2022 at 2:53 PM Dan Smith <daniel.smith at oracle.com> wrote:
>>
>>> Yes, a public annotation was the original proposal. At some point we scaled that back to just JDK-internal. The discussions were a long time ago, but if I remember right the main concern was that a formalized, Java SE notion of "value-based class" would lead to some unwanted complexity when we eventually get to *real* value classes (e.g., a misguided CS 101 course question: "what's the difference between a value-based class and a value class? which one should you use?").
>>
>>
>> Yeah, I hear that. The word "value" does have multiple confusable meanings. I'd say the key difference is that "value semantics" are logically a *recursive* rejection of identity, while a Valhalla B2/B3 class on its own addresses only one level deep.
>>
>> Anyway, I think what I'm proposing avoids trouble by specifically labeling one state as simply the transitional state to the other. I'm not sure there'd be much to get hung up on.
>>
>>
>>>
>>> It seemed like producing some special warnings for JDK classes would address the bulk of the problem without needing to fall into this trap.
>>
>>
>> I'd just say it addresses a more specific problem: how *those* particular classes can become B2/B3 (non-identity) classes.
>>
>>
>>>
>>> Would an acceptable compromise be for a third-party tool to support its own annotations, while also recognizing @jdk.internal.ValueBased as an alternative spelling of the same thing?
>>
>>
>> I think it's "a" compromise :-), I will just have to work through how acceptable.
>>
>> Is there any such thing as a set of criteria for when a warning deserves to be handled by javac instead of left to all the world's aftermarket static analyzers to handle?
>>
>>> (Secondarily... why are we warning only on synchronization, and not on `==` or (marginal) `identityHC`?)
>>>
>>> I think this was simply not a battle that we wanted to fight—discouraging all uses of '==' on type Integer, for example.
>>
>>
>> Who would be fighting the other side of that battle? Not anyone having some need to use `==` over `.equals()`, because we'll be breaking them when Integer changes buckets anyway. So... just the users saying "we should get to use `==` as a shortcut for `.equals()` as long as we stay within the cached range"? Oh, wait:
>>
>>
>>> Within these constraints, there are reasonable things that can be done with '==', like optimizing for a situation where 'equals' is likely to be true.
>>
>>
>> Ok, that too. Fair I suppose... it's just that it's such a very special case...
>>
>> --
>> Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb at google.com
>
>
>
> --
> Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb at google.com