8366178: Implement JEP 526: Lazy Constants (Second Preview) #27605
Conversation
I've updated the JEP. Thanks for pointing out this. Let me know what you think about the new text. |
The change was made to the wrong row (“Non‑ |
|
We are exploring better ways to provide low-level, imperative, lazy constant/stable value fields. This work will be done outside the current JEP. |
Webrevs
|
| */ | ||
|
|
||
| /* @test | ||
| * @summary Demo of an imperative stable value based on a lazy constant |
There was a problem hiding this comment.
Should this summary be updated (and. some of the classes defined here)
|
|
||
| } | ||
|
|
||
| interface LenientList<E> extends List<E> { |
There was a problem hiding this comment.
Ok, this seems needed to give access to getAcquire from sublists created from other sublists...
| import java.util.function.Supplier; | ||
|
|
||
| /** | ||
| * A lazy constant is a deferred, shallowly immutable constant to be computed at |
There was a problem hiding this comment.
It seems like the javadoc is a snapshot of an earlier version of the JEP where the word "content" was replaced with "constant". I think we should go back to "content" and say (as we did for stable values) that a "A lazy constant is a holder of contents that can be set at most once."
| * <p> | ||
| * The computing function runs on the caller’s thread. | ||
| * <p> | ||
| * If a thread that is blocked by another computing thread is interrupted, this is not |
There was a problem hiding this comment.
I'm not sure I understand what you mean in this para
There was a problem hiding this comment.
E.g. "thread that is blocked by another computing thread". Did you mean to say "if the thread running the computing function is interrupted"?
There was a problem hiding this comment.
If a thread A won the race and invokes the computing function, and another thread B comes along while computation takes place by A, then interrupting thread B will not have any effect.
| * <p> | ||
| * The {@code LazyConstant} type is not {@link Serializable}. | ||
| * <p> | ||
| * It is not recommended putting lazy constants into equality-based collections |
There was a problem hiding this comment.
Not sure what you mean here. It seems like the javadoc for equals says that it triggers initialization, so it should be ok?
There was a problem hiding this comment.
Formally, it is ok, but may result in strange side effects if we, for example, put a key of type LazyConstant in a map. It will then be initialized immediately (upon map.put()), and the put operation can therefore have non-obvious side effects.
There was a problem hiding this comment.
I think it would be better to link to equals more explicitly -- e.g. "As equals and hashCode might trigger the initialization of a lazy constant, it is recommended to avoid storing lazy constants into equality-based collections (or similar constructs) as their addition (e.g. via Map::put, or Set::add) might result in subtle side-effects).
| * {@code obj}, if it is an instance of a {@linkplain LazyConstant}. Consequently, | ||
| * this method might block or throw. | ||
| * | ||
| * @implSpec The order of potential initialization triggering is specified as: |
There was a problem hiding this comment.
Do we really need to specify this?
There was a problem hiding this comment.
This is a corner case, but we might have two uninitialized LCs. The order in which they get computed might be significant (e.g., they operate on shared data structures).
There was a problem hiding this comment.
I understand that the order might be significant -- but I still wonder if we should specify that order. This seems to add constraints to the implementation. Do we really want clients to rely on the order in which lazy constants are initialized when calling equals ?
Co-authored-by: Maurizio Cimadamore <[email protected]>
| * the initialized constant is read. Hence, the initialized constant, including any | ||
| * {@code final} fields of any newly created objects, are safely published. | ||
| * <p> | ||
| * If the computing thread or any thread that is blocked by the computing thread |
There was a problem hiding this comment.
My worry here is that when we say "any thread that is blocked" we really meany "any thread". But in reality this javadoc only refers to threads that are trying to call get and end up blocked because some other thread won the race. E.g. if a thread is blocked by the computing thread for any other reason, we don't know whether interrupting that thread will result in an InterruptedException ? I think it might be worth simplifying this a bit, starting with the property you want to assert:
Thread interruption does not cancel initialization of a lazy constant. In other words, if the computing thread is
interrupted,LazyConstant::getdoesn't clear the interrupted thread’s status is not cleared, nor does it throw
anInterruptedException.
| * otherwise {@code false} | ||
| */ | ||
| @Override | ||
| boolean equals(Object obj); |
There was a problem hiding this comment.
There is a tension here (same for hashCode). A lazy constant is a mutable cell that can be updated only once, given some computing function. When you compare two lazy constants, you can either compare the mutable cell (e.g. the pointer to the memory location where the constant will be eventually stored), or you can compare the constants. Here, the javadoc decides to opt for comparing the constants -- but this might be problematic, as now equals can throw exceptions too (and/or result in blocking, as you say in the javadoc). So, I'm not too sure what's the best option here -- do we have an idea of how frequent it is to want to compare two lazy constants "by value" ?
(for reference, we have no precedent for this: ClassValue, ScopedValue and ThreadLocal do not redefine equals/hashCode).
| @Stable | ||
| transient Set<K> keySet; |
There was a problem hiding this comment.
Are all uses of this field in java.util able to handle the @Stableness of this field correctly?
8 participants
Implement JEP 526: Lazy Constants (Second Preview)
The lazy list/map implementations are broken out from
ImmutableCollectionsto a separate class.The old benchmarks are not moved/renamed to allow comparison with previous releases.
java.util.Optionalis updated so that its field is annotated with@Stable. This is to allowOptionalinstances to be held in lazy constants and still provide constant folding.Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27605/head:pull/27605$ git checkout pull/27605Update a local copy of the PR:
$ git checkout pull/27605$ git pull https://git.openjdk.org/jdk.git pull/27605/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27605View PR using the GUI difftool:
$ git pr show -t 27605Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27605.diff
Using Webrev
Link to Webrev Comment