Re: RFR: 8242888: Convert dynamic proxy to hidden classes
> From: "-" > To: "Remi Forax" > Cc: "Chen Liang" , "core-libs-dev" > , "hotspot-dev" , > "kulla-dev" > Sent: Thursday, May 23, 2024 2:56:58 PM > Subject: Re: RFR: 8242888: Convert dynamic proxy to hidden classes > Hmm, I think Proxy being hidden in stacktraces might be an advantage; the same > happens for lambdas. > The main advantage of hidden classes compared to an explicit class with > classData is that it supports flexible unloading, which might be useful for > Proxy. Flexible unloading has a high cost in term of memory, the class + methods, etc need their own metaspace. While on paper it seems great, I've my doubt that it's a good idea to use that option for proxies given that the Proxy API allows an umbounded number of proxy classes. That's why lambda proxies does not use the flexible unloading anymore. > I still believe the flexible unloading advantage justifies the migration to > hidden classes. > Chen Rémi > On Thu, May 23, 2024 at 6:43 AM Remi Forax < [ mailto:fo...@univ-mlv.fr | > fo...@univ-mlv.fr ] > wrote: >> - Original Message - >> > From: "Chen Liang" < [ mailto:li...@openjdk.org | li...@openjdk.org ] > >>> To: "core-libs-dev" < [ mailto:core-libs-dev@openjdk.org | >>> core-libs-dev@openjdk.org ] >, "hotspot-dev" < [ >>> mailto:hotspot-...@openjdk.org >>> | hotspot-...@openjdk.org ] >, [ mailto:kulla-...@openjdk.org | >> > kulla-...@openjdk.org ] >> > Sent: Thursday, May 23, 2024 1:28:01 PM >> > Subject: Re: RFR: 8242888: Convert dynamic proxy to hidden classes >>> On Thu, 23 May 2024 03:28:30 GMT, Chen Liang < [ mailto:li...@openjdk.org | >> > li...@openjdk.org ] > wrote: >> >> Please review this change that convert dynamic proxies implementations to >> >> hidden >> >> classes, intended to target JDK 24. >> >> Summary: >> >> 1. Adds new implementation while preserving the old implementation behind >> >> `-Djdk.reflect.useLegacyProxyImpl=true` in case there are compatibility >> >> issues. >> >> 2. ClassLoader.defineClass0 takes a ClassLoader instance but discards it >> >> in >> >> native code; I updated native code to reuse that ClassLoader for Proxy >> >> support. >> >> 3. ProxyGenerator changes mainly involve using Class data to pass Method >> >> list >> >> (accessed in a single condy) and removal of obsolete setup code >> >> generation. >> >> Testing: tier1 and tier2 have no related failures. >> >> Comment: Since #8278, Proxy has been converted to ClassFile API, and >> >> infrastructure has changed; now, the migration to hidden classes is much >> >> cleaner and has less impact, such as preserving ProtectionDomain and >> >> dynamic >> >> module without "anchor classes", and avoiding java.lang.invoke package. >> > A CSR targeting 24 describing the compatibility concerns and behavioral >> > differences is here, somehow not linked by skara: >>> [ https://bugs.openjdk.org/browse/JDK-8332770 | >> > https://bugs.openjdk.org/browse/JDK-8332770 ] >> > The incompatibilities were much greater in the previous iterations of this >> > issue, such as in dynamic modules, serialization, and in proxy class >> > protection >> > domain. Now these aspects are addressed by this patch, the only real one >> > left >> > is the change in stack trace. Feel free to raise other incompatibilities >> > you >> > have discovered. >> I wonder if instead of using hidden classes, we should not use usual named >> classes and add a new Lookup.defineClass() that takes a classData as >> parameter. >> This will solve the both the problem of the stacktrace and the problem of the >> roundtrip proxyClass != Class.forName(proxyClass.getName()). >> Rémi >> > - >>> PR Comment: [ >>> https://git.openjdk.org/jdk/pull/19356#issuecomment-2126869679 | >> > https://git.openjdk.org/jdk/pull/19356#issuecomment-2126869679 ]
Re: [External] : Gatherer and primitive specialization
> From: "Viktor Klang" > To: "Remi Forax" , "core-libs-dev" > > Sent: Thursday, June 13, 2024 12:52:03 PM > Subject: Re: [External] : Gatherer and primitive specialization > Hi Rémi, > Given that Collector has not been specialized since it was introduced, we > opted > to not specialize Gatherer eagerly as Valhalla Value Classes may also move the > needle a bit regarding the need for specialization in general. As i said previously, most collectors uses collections so until collections are specialized, specializing the collector API is not that useful. A gatherer unlike a Collector can just transform values, thus specializing it is more useful. For Valhalla, we are at year 10 and value classes are not there yet, specialization of generics is in my opinion 10 years ahead. I'm not complaining, if we had rush value classes, everybody will have regret it, but those things take a lot of time. > It was also not clear how far to take such specialization—since Gatherers have > both input types and output types, should they be specialized in two axes or > only one? If it's like mapMulti, it's the two axis, you have 4 x 4 possible types (Object,int, long, double). And because the integrator uses an interface for being greedy or not, it's 16 times 2. > It also has implications on composition of Gatherers. If we have something like a Gatherer.ofInt() (like with Iterator or Spliterator), we need 4 andThen() (or one andThen with a pattern matching with 4 cases) so it has implications but it's not a road block. > Cheers, > √ regards, Rémi > Viktor Klang > Software Architect, Java Platform Group > Oracle > From: Remi Forax > Sent: Thursday, 13 June 2024 10:16 > To: core-libs-dev > Cc: Viktor Klang > Subject: [External] : Gatherer and primitive specialization > Hello, > the Gatherer API do not provide primitive specialization while intermediate > methods like map(), flatMap() or mapMulti() do. > Unlike collectors where it is okay to not be specialized because most > collectors > mutate a collection that is not specialized too, > gatherers can process/transform values so having boxing in the middle of those > kind of computations is a performance pothole. > I think there is a trick that can be used, we do not need to specialize all > the > functions of the Gatherer API but only the integrator. > All other functions are called a few time when a stream is processed so they > may > not need specialization, only the integrator is called a lot. > regards, > Rémi
Re: [External] : Re: New candidate JEP: 431: Sequenced Collections
- Original Message -
> From: "Stuart Marks"
> To: "Remi Forax"
> Cc: "core-libs-dev"
> Sent: Sunday, October 16, 2022 12:25:18 AM
> Subject: Re: [External] : Re: New candidate JEP: 431: Sequenced Collections
> Hi Rémi,
>
> On 10/14/22 1:20 AM, Remi Forax wrote:
>> People will again think that i'm the grumpy guy but i prefer to voice my
>> concerns.
>>
>> - nobody cares, i'm back from Devoxx and nobody cares about Sequenced
>> Collections, i've tried to ask several friends what they think about it and
>> the
>> answer was "meh".
>>The bar to introduce new interfaces in the collection API is really
>> really high
>>given how the Java ecosystem works.
>>Once a library starts to use those interfaces as method parameter, it
>> pressures
>>the other library authors to write methods that provides object typed as
>> those
>>interfaces.
>>Not enough people care and the cost for the community (not only Oracle)
>> is high,
>>it looks like a recipe for failure.
>
> Fortunately, we don't make decisions in OpenJDK via opinion polls. :-)
We still have users tho.
>
> I'm obviously not in a position to address the concerns of your interlocutors.
> However, the history in the bug database and the recurring discussions on
> core-libs-dev (some of which are linked in the JEP) show the need for this.
There are several feature requests that can be bundled into this JEP, but that
not the only ways to implement those requests.
>
> Introduction of new types always poses a dilemma for library maintainers. They
> can
> move forward aggressively and embrace new features, or they can remain on
> older
> releases in order to reach a broader audience. That's not a reason to avoid
> introducing new types.
I agree in absolute, but in this specific case you want to introduce interfaces
deep into the existing hierarchy of interfaces, not on the top.
The introduction of CharSequence has been painful and very slow, that interface
was added in 1.4 but we had to wait 9 more than 10 years later to have
parseInt() that takes a CharSequence.
As an application developer, a decade is the kind of time i will have to wait
before i can use a sequenced collection.
Adding default methods is far faster in term of adoption, computeIfAbsent or
removeIf were available at the time of the release of 1.8.
Default methods have issues too, it only works if a good default implementation
exists.
>
>> - LinkedHashMap can be tweaked in two ways, by passing an access order as 3rd
>> parameter of the constructor or by overriding removeEldesEntry(), in both
>> cases
>> the resulting LinkedHashMap is at odds with the contract of SequencedMap but
>> the compiler will happily allow to see those LinkedHashMap as SequencedMap.
>
> SequencedMap specifies that entries have an encounter order but it doesn't
> make
> any
> requirements on how that order is established. LinkedHashMap's access-order
> mode
> is
> unusual in that it specifies that specific methods additionally have the side
> effect
> of reordering the relevant entry. The removeEldestEntry() method modifies the
> behavior of mapping-insertion methods to optionally remove the first
> ("eldest")
> entry. Neither of these behaviors conflicts with anything in SequencedMap.
It conflicts with the idea of the first element (or last element)
SequencedSet set = ...
var first = set.getFirst();
assertNotNull(first);
set.add("foo");
var first2 = set.getFirst();
assertEquals(first, first2); // should always be true, right ?
>
>> - LinkedHashMap/LinkedHashSet are dinosaurs, there are more efficient
>> implementations of the concepts of ordered set / ordered map both in term of
>> memory footprint and runtime execution, so adding new interfaces without
>> exploring new implementations using Valhalla value class and in relation with
>> the Collection Literal JEP seems premature to me.
>
> LinkedHashMap and LinkedHashSet are in fact used fairly frequently. They're
> not
> used
> as much as HashMap, but they're used more than ArrayDeque or TreeMap.
> Presumably
> this is because people find LinkedHashMap/Set useful and that the overhead
> compared
> to their non-linked counterparts is acceptable for the additional services
> they
> provide.
>
> The performance and footprint of LinkedHashMap/Set are not impacted either way
> by
> the JEP, nor are any potential future performance improvements (including
> those
> that
> might arise because of Valhalla) precluded by the JEP.
I don't disagree with all you
Re: [External] : Re: New candidate JEP: 431: Sequenced Collections
- Original Message - > From: "Stuart Marks" > To: "Remi Forax" > Cc: "core-libs-dev" > Sent: Saturday, October 29, 2022 2:16:06 AM > Subject: Re: [External] : Re: New candidate JEP: 431: Sequenced Collections [...] > > You seem to be talking about introduction of new types in opposition to > introduction > of default methods. This proposal does both. Clearly if you're using an old > library, > you can't use new types with it. But if the old library consumes or produces > say, a > List, new applications can definitely use all the default methods added to > List > by > this proposal. > > We've previously discussed adding new default methods (getFirst, reversed) to > some > existing type (like Collection) instead of on a new type. The problem is that > they > appear on all collections, even those that have no encounter order. Their > specifications would thus be so weak as to be meaningless. That's the > justification > for adding a new type: to give these methods meaning. All collections have an order the one of their iterator. [...] > > There is a definition of encounter order, but there is no specification of how > the > encounter order is established or how it's modified. That's left to > implementations and subtypes. How this is different to saying the encounter order is the one of the iterator ? > > This example 1) gets the first element, 2) adds another element, and 3) gets > the > first element again. Step 2 is a modification to the collection, so there's no > basis > for asserting that the element obtained in step 1 always equals the element > obtained > in step 3. The example shows that add() does not add() at the end of a SequencedSet, the order defined SequencedSet is at best surprising, it's all i'm saying. That why i think SequencedCollection as an interface does not worth the trouble because you can not offer more guarantees than saying the order is the order of the iterator. But at least if the operations are defined on java.util.Collection, people will not fall into the trap of thinking that "sequenced" means that add() adds at the end of the sequence. > >>>> - LinkedHashMap/LinkedHashSet are dinosaurs, there are more efficient >>>> implementations of the concepts of ordered set / ordered map both in term >>>> of >>>> memory footprint and runtime execution, so adding new interfaces without >>>> exploring new implementations using Valhalla value class and in relation >>>> with >>>> the Collection Literal JEP seems premature to me. >>> >>> LinkedHashMap and LinkedHashSet are in fact used fairly frequently. They're >>> not >>> used >>> as much as HashMap, but they're used more than ArrayDeque or TreeMap. >>> Presumably >>> this is because people find LinkedHashMap/Set useful and that the overhead >>> compared >>> to their non-linked counterparts is acceptable for the additional services >>> they >>> provide. >>> >>> The performance and footprint of LinkedHashMap/Set are not impacted either >>> way >>> by >>> the JEP, nor are any potential future performance improvements (including >>> those >>> that >>> might arise because of Valhalla) precluded by the JEP. >> >> I don't disagree with all you say above, i'm saying that with Valhalla, we >> need >> to explore implementations of HashMap based on open addressing to get better >> cache performance. >> So adding addFirst() to LinkedHashMap now, is a risk. > > As the LinkedHashMap implementation stands today (prior to JEP 431) it needs > to > perform a variety of operations that affect the order of the elements. The > addFirst() method is a new operation, but it's only a small amount of > additional > work, given that there are already other operations that operate on both ends. > > In a hypothetical future open-addressed Map, there would need to be some > additional > data structure (likely an array?) that maintains the encounter order. This > structure > would need to support various element shifting and copying operations to > maintain > the ordering; consider what would need to happen with a get() operation on an > access-ordered map. I don't see any problem with implementing an addFirst() > operation on this structure. You do not need any shifting if either you use a tombstone in the array that maintain the encounter order (you can clean them at the same time you rehash) or if you define that removing an entry replace it by the last e
Re: The introduction of Sequenced collections is not a source compatible change
Another example sent to me by a fellow French guy,
final Deque nestedDequeue = new ArrayDeque<>();
nestedDequeue.addFirst("C");
nestedDequeue.addFirst("B");
nestedDequeue.addFirst("A");
final List nestedList = new ArrayList<>();
nestedList.add("D");
nestedList.add("E");
nestedList.add("F");
final List> list = Stream.of(nestedDequeue,
nestedList).toList();
This one is cool because no 'var' is involved and using
collect(Collectors.toList()) instead of toList() solves the inference problem.
Rémi
- Original Message -
> From: "Stuart Marks"
> To: "Remi Forax"
> Cc: "core-libs-dev"
> Sent: Tuesday, May 2, 2023 2:44:28 AM
> Subject: Re: The introduction of Sequenced collections is not a source
> compatible change
> Hi Rémi,
>
> Thanks for trying out the latest build!
>
> I'll make sure this gets mentioned in the release note for Sequenced
> Collections.
> We'll also raise this issue when we talk about this feature in the Quality
> Outreach
> program.
>
> s'marks
>
> On 4/29/23 3:46 AM, Remi Forax wrote:
>> I've several repositories that now fails to compile with the latest jdk21,
>> which
>> introduces sequence collections.
>>
>> The introduction of a common supertype to existing collections is *not* a
>> source
>> compatible change because of type inference.
>>
>> Here is a simplified example:
>>
>>public static void m(List>>
>> factories) {
>>}
>>
>>public static void main(String[] args) {
>> Supplier> supplier1 = LinkedHashMap::new;
>> Supplier> supplier2 = TreeMap::new;
>> var factories = List.of(supplier1, supplier2);
>> m(factories);
>>}
>>
>>
>> This example compiles fine with Java 20 but report an error with Java 21:
>>SequencedCollectionBug.java:28: error: method m in class
>> SequencedCollectionBug
>>cannot be applied to given types;
>> m(factories);
>> ^
>>required: List>>
>>found:List>>
>>reason: argument mismatch; List> SequencedMap>>
>>cannot be converted to List>>
>>
>>
>>
>> Apart from the example above, most of the failures I see are in the unit
>> tests
>> provided to the students, because we are using a lot of 'var' in them so they
>> work whatever the name of the types chosen by the students.
>>
>> Discussing with a colleague, we also believe that this bug is not limited to
>> Java, existing Kotlin codes will also fail to compile due to this bug.
>>
>> Regards,
> > Rémi
Re: The introduction of Sequenced collections is not a source compatible change
Hi Joe, in this peculiar case, there are several reasons to be worried compared to other potential breaking changes that has appeared in the past (see the message of Tagir for an example). Unlike other changes - this one touch the collection API, and those interfaces/types are widely used, - we know that the source compatibility changes occurs mostly if 'var' or the "new" inference algorithm (the one from Java 8), so this is likely that most of the issues will be found in Java 11+ source code, - this changes may also affect all typed languages based on the JVM, not only Java. Corpus of codes in Groovy, Kotlin and Scala also need to be checked. In case of Kotlin and Scala, 'var' is the default behavior but they have their own collections (or type system around collections in case of Kotlin), so knowing the real impact of this change is hard here. The problem of using a corpus experiment is that the corpus may not represent the current state of the Java ecosystem, or at least the one that may be impacted. In my case, on my own repositories (public and private), i had only one occurrence of the issue in the main source codes because many of those repositories are not using 'var' or even the stream API but on the corpus of the unit tests we give to students to check their implementations, little less than a third of those JUnit classes had source compatibility issues because those tests are using 'var' and different collections heavily. And the situation is a little worst than that because in between now and the time people will use Java 21, a lot of codes will be written using Java 11 and 17 and may found incompatible later. A source incompatibility issue is not a big deal, as said in this thread, most of the time, explicitly fixing the type argument instead of inferring it make the code compile again. So the house is not burning, but we should raise awareness of this issue given that it may have a bigger impact than other source incompatible changes that occur previously. Rémi > From: "joe darcy" > To: "Ethan McCue" , "Raffaello Giulietti" > > Cc: "Remi Forax" , "Stuart Marks" > , > "core-libs-dev" > Sent: Friday, May 5, 2023 4:38:16 AM > Subject: Re: The introduction of Sequenced collections is not a source > compatible change > A few comments on the general compatibility policy for the JDK. Compatibility > is > looked after by the Compatibility and Specification Review (CSR) process ( > Compatibility & Specification Review). Summarizing the approach, >> The general compatibility policy for exported APIs implemented in the JDK is: >> * Don't break binary compatibility (as defined in the Java Language >> Specification) without sufficient cause. >> * Avoid introducing source incompatibilities. >> * Manage behavioral compatibility changes. > [ https://wiki.openjdk.org/display/csr/Main | > https://wiki.openjdk.org/display/csr/Main ] > None of binary, source, and behavioral compatibly are absolutes and judgement > is > used to assess the cost/benefits of changes. For example, strict source > compatibility would preclude, say, introducing new public types in the > java.lang package since the implicit import of types in java.lang could > conflict with a same-named type *-imported from another package. > When a proposed change is estimated to be sufficiently disruptive, we conduct > a > corpus experiment to evaluate the impact on the change on many public Java > libraries. Back in Project Coin in JDK 7, that basic approach was used to help > quantify various language design choices and the infrastructure to run such > experiments has been built-out in the subsequent releases. > HTH, > -Joe > CSR Group Lead > On 5/4/2023 6:32 AM, Ethan McCue wrote: >> I guess this a good time to ask, ignoring the benefit part of a cost benefit >> analysis, what mechanisms do we have to measure the number of codebases >> relying >> on type inference this will break? >> Iirc Adoptium built/ran the unit tests of a bunch of public repos, but it's >> also >> a bit shocking if the jtreg suite had nothing for this. >> On Thu, May 4, 2023, 9:27 AM Raffaello Giulietti < [ >> mailto:raffaello.giulie...@oracle.com | raffaello.giulie...@oracle.com ] > >> wrote: >>> Without changing the semantics at all, you could also write >>> final List> list = >>> Stream.>of(nestedDequeue, nestedList).toList(); >>> to "help" type inference. >>> On 2023-05-03 15:12, [ mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ] wrote: >>> > Another example sent to me by a fellow French guy, >>> > final Deque nestedD
Re: JEP 442: Foreign Function & Memory API => why is it again preview API?
- Original Message - > From: "Uwe Schindler" > To: "Remi Forax" > Cc: "core-libs-dev" > Sent: Wednesday, May 17, 2023 9:13:35 AM > Subject: Re: JEP 442: Foreign Function & Memory API => why is it again > preview API? > Hi Remi, > > thanks for the reponse! > >>> yesterday Apache Lucene got the information that JDK 21 got the project >>> panama JEP 442 update and I implemented it already in our source tree. >>> >>> Unfortunately the API is again marked "preview", but JDK 21 is "LTS >>> release". Many of our users (Elasticserach, Solr) will be switching to >>> this version. We were really hoping that the java.lang.foreign API is >>> finished at that time. I checked the changes in our code: just a rename >>> of a method and FileChannel#map now takes Arena instead of Scope. >> I see that Alan and Maurizio have already answer to your other points. >> >> Having preview features and being a LTS are to separate concerns. >> Being a LTS is about support, having preview features is about having >> feedback >> before finalizing an API. >> >> Java 17 was released with preview features, Java 21 will be. > > Yes that's the case. But Java 17 did not have any "runtime visible" > preview features, only compiler had preview features. For public > libraries out there (open source on Maven central like Lucene), people > don't care how they are produced by a compiler. So enhanced switch > statements or similar stuff is a bit different than the current problem > where the preview features are visible at runtime. Not true, most of the language changes in preview also have a corresponding runtime visible API marked as preview API. In case of jdk17, the SwitchBootstraps API is marked as preview API https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/runtime/SwitchBootstraps.html > > Some related note: I don't understand why classes compiled with preview > SYNTAX features of the Java compiler need to be marked with the preview > bit; IMHO compiler outputs should only be marked as preview if they use > preview APIs. There should be no risk in running the compiler output of > preview switch or instanceof statements without preview features enabled. Because a syntax feature usually also requires a support API, either at runtime or at compile time :) Records has java.lang.Record, Text blocks has String.stripIndent, etc > > Uwe Rémi > > -- > Uwe Schindler > uschind...@apache.org > ASF Member, Member of PMC and Committer of Apache Lucene and Apache Solr > Bremen, Germany > https://lucene.apache.org/ > https://solr.apache.org/
Re: Proposal: Collection mutability marker interfaces
> From: "Ethan McCue"
> To: "Remi Forax"
> Cc: "John Hendrikx" , "core-libs-dev"
>
> Sent: Wednesday, August 24, 2022 4:27:01 PM
> Subject: Re: Proposal: Collection mutability marker interfaces
>> so it's perhaps better to call add() inside a try/catch on
> > UnsupportedOperationException.
> As much as sin is wrong, sketching out what that might look like... forgive
> the
> toyness of the example
> VS
> final class Ex {
> private Ex() {}
> /*
> * Adds the odd numbers from 1 to 10 to the List then makes all the odds even.
> *
> * Takes ownership of the list, avoids making copies if it doesn't need to
> */
> static List addOdds(List l) {
> for (int i = 1; i <= 10; i++) {
> try {
> l.add(i);
> } catch (UnsupportedOperationException e) {
> l = new ArrayList<>(l);
i -= 1; // restart with an ArrayList
> }
> }
> for (int i = 0; i < l.size(); i++) {
> if (l.get(i) % 2 == 1) {
> try {
> l.set(i, l.get(i) + 1);
> } catch (UnsupportedOperationException e) {
> l = new ArrayList<>(l);
> }
> }
> }
> }
> }
as Roger said, there is no way in Java to know if the caller has not kept a
reference (unlike Rust),
so having trouble to write this kind of code is more a feature than an issue :)
This kind of examples scream the Stream API, which has the correct semantics
IntStream.rangeClosed(1, 10).map(i -> i % 2 == 0? i + 1: i).boxed().toList()
Rémi
> On Wed, Aug 24, 2022 at 10:03 AM Remi Forax < [ mailto:fo...@univ-mlv.fr |
> fo...@univ-mlv.fr ] > wrote:
>>> From: "Ethan McCue" < [ mailto:et...@mccue.dev | et...@mccue.dev ] >
>>> To: "John Hendrikx" < [ mailto:john.hendr...@gmail.com |
>>> john.hendr...@gmail.com
>>> ] >
>>> Cc: "core-libs-dev" < [ mailto:core-libs-dev@openjdk.org |
>>> core-libs-dev@openjdk.org ] >
>>> Sent: Wednesday, August 24, 2022 3:38:26 PM
>>> Subject: Re: Proposal: Collection mutability marker interfaces
>>> A use case that doesn't cover is adding to a collection.
>>> Say as part of a method's contract you state that you take ownership of a
>>> List.
>>> You aren't going to copy even if the list is mutable.
>>> Later on, you may want to add to the list. Add is supported on ArrayList so
>>> you
>>> don't need to copy and replace your reference, but you would if the list you
>>> were given was made with List.of or Arrays.asList
>> You can ask if the spliterator considers the collection as immutable or not,
>> list.spliterator().hasCharacteristics(Spliterator.IMMUTABLE)
>> sadly, List.of()/Map.of() does not report the spliterator characteristics
>> correctly (the spliterator implementations are inherited from
>> AbstracList/AbstractMap).
>> so it's perhaps better to call add() inside a try/catch on
>> UnsupportedOperationException.
>> Rémi
>>> On Wed, Aug 24, 2022, 8:13 AM John Hendrikx < [
>>> mailto:john.hendr...@gmail.com |
>>> john.hendr...@gmail.com ] > wrote:
>>>> Would it be an option to not make the receiver responsible for the decision
>>>> whether to make a copy or not? Instead put this burden (using default
>>>> methods)
>>>> on the various collections?
>>>> If List/Set/Map had a method like this:
>>>> List immutableCopy(); // returns a (shallow) immutable copy if list is
>>>> mutable (basically always copies, unless proven otherwise)
>>>> Paired with methods on Collections to prevent collections from being
>>>> modified:
>>>> Collections.immutableList(List)
>>>> This wrapper is similar to `unmodifiableList` except it implements
>>>> `immutableCopy` as `return this`.
>>>> Then for the various scenario's, where `x` is an untrusted source of List
>>>> with
>>>> unknown status:
>>>> // Create a defensive copy; result is a private list that cannot be
>>>> modified:
>>>> List y = x.immutableCopy();
>>>> // Create a defensive copy for sharing, promising it won't ever change:
>>>> List y = Collections.immutableList(x.immutableCopy());
>>>> // Create a defensive copy for mutating:
>>>> List y = new ArrayList<>(x); // same as always
>>>> // Create a mutable copy, modify it, then expose as immutable:
>>>> List y = new ArrayList<>(x); // same as always
>>>> y.add( );
>>>> List z = Collections.immutableList(y);
>>>> y =
Re: java.util.stream.Stream: API for user-extensible intermediate operations
> From: "Viktor Klang" > To: "Remi Forax" > Cc: "core-libs-dev" > Sent: Thursday, June 29, 2023 11:09:22 AM > Subject: Re: java.util.stream.Stream: API for user-extensible intermediate > operations > From: Remi Forax > Sent: Thursday, 29 June 2023 10:03 > To: Viktor Klang > Cc: core-libs-dev > Subject: [External] : Re: java.util.stream.Stream: API for user-extensible > intermediate operations >> From: "Viktor Klang" >> To: "core-libs-dev" >> Sent: Tuesday, June 27, 2023 7:10:42 PM >> Subject: java.util.stream.Stream: API for user-extensible intermediate >> operations >> Hi core-libs-dev, >> Over the past 6+ months I've been thinking about, and tinkering with, how >> we'd >> be able to expose a user-facing API for extensible intermediate >> java.util.stream.Stream operations—a feature envisioned all the way back when >> Streams were created. >> I'm now at a point where I have a viable design and implementation, and so >> I'm >> turning to you for your feedback: on the direction taken; the API concepts; >> and, in particular, is there anything which I have overlooked/missed? >>I think this API is overly generic and hard to reason about it, for users and > >IDEs. > The API is for all intents and purposes Collector with a boolean return type > for > the accumulator and an added downstream handle parameter added to the > accumulator and the finisher. >>The main issue is that the same API is used for both stateless and stateful >>operations, which means that as a user, we have no idea if a call to > >stream.gather() is stateful or not. > How is this different from any of the other pre-existing Stream operations? Most of the operation are stateless, only a handful of well known operations are stateful, all other stateful operations are done by collectors. But stream.gather() allows both stateless and stateful gatherer. I believe that instead of having an intermediary operation that can be stateless or stateful, it's better to have a Collector that starts a new stream. Instead of stream.gather(Gatherers.foo()) A collector/gatherer can propagate the elements into a new stream stream.collect(Gatherers.foo(stream -> ...)) or stream.collect(Gatherers.foo(), stream -> ...) This allows a better control on the parallelization (both streams are independant) and a clear path to retrofit the Collectors as Gatherers (instead of having two too similar APIs side by side). >>Which is a departure from the current API that cleanly separate stateless and >>staful operations. Here, we are left in the dark. In a sense, this API is too > >powerful, it can do too much thing, so as a user we can not reason about it. > A Gatherer encodes it input and output types, in what sense would that not be > enough to reason about it ? The initial idea of the API is to have almost all intermediary operations to be stateless so by default, we know that the complexity of the stream is linear, it will parallelize quite well, etc. Once you have an intermediary operation like a gatherer, all this "good" property are null and void. >>I like the idea of a Collector 2.0 i.e. using the Gatherer API at the end of >>the >>stream (not in the middle), but currently, the Gatherer API is not a >>Collector, >>so we now have two different APIs for doing partially the same job. I wonder >>if >>the Collector API can be retroffitted to act as a Gatherer API, avoiding to >>have to choose which one to use, a gatherer being the equivalent of a > >"flat-collector" + short-circuit. > Collector serves a very important role of being able to get information out > of a > Stream and deliver that information in a certain shape, a Gatherer does not > provide any facility for this. A collector can get information out of a Stream into a new one , at that point you have something quite similar to a Gatherer. >>The idea of unsupportedCombiner() seems out of place, like a patch to be able >>to >>clobble different things together. I'm not sure to understand why it's needed > >for a Gatherer, and why it is not needed for Collectors ? > Nothing prevents us from treating a `null` combiner the same way. My primary > reason for making it a dedicated thing was to be able to differentiate a > possible bug (user passing in a null reference inadvertently) from explicitly > stating that a combiner does not exist from this operation. > unsupportedCombiner() as an artifact can be completely hidden if desired, as > Gatherer.of() can have permutations without specifying a combiner, and the > default method of Gatherer.comb
Re: Gatherer API : wildcards complaint
> From: "Viktor Klang"
> To: "Remi Forax" , "core-libs-dev"
>
> Sent: Wednesday, January 17, 2024 5:49:01 PM
> Subject: Re: Gatherer API : wildcards complaint
> Hi Rémi,
> Thank you for the feedback—examples would be much appreciated!
Here is an example with an interface and a class,
interface Counter {
void increment ();
int value ();
}
Gatherer < String , Counter , Integer > count () {
class CounterImpl implements Counter {
int counter ;
@Override
public void increment () {
counter ++;
}
@Override
public int value () {
return counter ;
}
}
Supplier < CounterImpl > initializer = CounterImpl :: new ;
Gatherer . Integrator < Counter , String , Gatherer . Downstream > integrator = ( counter , _ , _ ) -> {
counter .increment();
return true ;
};
BiConsumer < Counter , Gatherer . Downstream > finisher = (
counter , downstream ) -> {
downstream .push( counter .value());
};
return Gatherer . ofSequential ( initializer , integrator , finisher ); // does
not compile :(
}
void main () {
System . out .println( Stream . of ( "foo"
).gather(count()).findFirst().orElseThrow());
}
if instead of explicitly typing each functions, we directly call ofSequential,
it works
return Gatherer . ofSequential (
CounterImpl :: new ,
( Counter counter , String _ , Gatherer . Downstream _ ) ->
{
counter .increment();
return true ;
},
( Counter counter , Gatherer . Downstream downstream ) -> {
downstream .push( counter .value());
}
);
because here, CounterImpl::new is inferred as Supplier.
> Cheers,
> √
> Viktor Klang
> Software Architect, Java Platform Group
> Oracle
> From: core-libs-dev on behalf of Remi Forax
>
> Sent: Wednesday, 17 January 2024 16:55
> To: core-libs-dev
> Subject: Gatherer API : wildcards complaint
> Hello,
> this is a minor complaint but I do not see a raison to not getting this right.
> Currently the Gatherer API does not use the wildcard correctly, which is not
> fully an issue because there is "enough" wildcards that if you rely on the
> inference, it will work.
> The problem is that when you write code, you make mistakes and usually when
> you
> have a typing issue, a way to debug it is to fix the type arguments
> de-activating the inference.
> But because there are some missing wildcards, this debugging strategy just
> fail
> flat with more typing errors.
> I think that fixing the missing wildcards will hep users (or a least me) to
> have
> a better experience when using the Gatherer API.
> I can help and propose a PR if you want.
> regards,
> Rémi
Re: Seing a Collector as a Gatherer
> From: "Viktor Klang"
> To: "Remi Forax"
> Sent: Wednesday, January 17, 2024 6:01:38 PM
> Subject: Re: Seing a Collector as a Gatherer
> Hi Rémi,
> Yes, this was something I was hoping to get into the preview, but I wasn't
> sure
> where that conversion should end up.
> There are a few different places where it might go:
> Gatherer.of(Collector)
> Gatherers.collect(Collector)
> Collector.asGatherer()
> Collectors.gather(Collector)
> I wasn't really convinced where it should go, and I was hesitant to making any
> changes to existing public interfaces as a "nice to have", so I opted to leave
> it out for now.
> I think people would prefer to see it on Collector as a default method, but
> as I
> said before, making changes to Collector wasn't something I was ready to
> prioritize for the (first) JEP.
I think this method is also important pedagogically, there should be a place
that describe the relationship between a Collector and a Gatherer.
For Gatherer.of(), this one seems alien compared to the other overloads of
of()/ofSequential() and to a lesser extend I do not like too much to have
overloads with one parameter with two different interfaces, because someone can
comes with a class that implements both Collector and Integrator (as stupid as
it is),
For Gatherers.collect(Collector) is fine,
For Collector.asGatherer(), a default method has the disadvantage that usually
a Collector is typed from left to right so calling an instance method requires
an intermediary variable
Collector> collector = Collector.toList(); // ok
Gatherer> gatherer = Collector.toList().asGatherer();
// we are in trouble here
that's why Collectors.collectingAndThen() (aka compose the finisher) is a
static method in Collectors and not an instance method in Collector
(finishAndThen ?),
For Collectors.gather(), methods inside Collectors tend to return a Collector.
> Cheers,
> √
regards,
Rémi
> Viktor Klang
> Software Architect, Java Platform Group
> Oracle
> From: core-libs-dev on behalf of Remi Forax
>
> Sent: Wednesday, 17 January 2024 17:08
> To: core-libs-dev
> Subject: Seing a Collector as a Gatherer
> Hello,
> I may have overlook that, but it seems there is no method to see a Collector
> as
> a Gatherer.
> A Gatherer is more general than a Collector and a Gatherer with a greedy
> integrator that does not call Downstream.push in the intergator and only once
> is the finisher is basicaly a Collector.
> In code:
> Gatherer asGatherer(Collector
> collector) {
> var supplier = collector.supplier();
> var accumulator = collector.accumulator();
> var combiner = collector.combiner();
> var finisher = collector.finisher();
> return Gatherer.of(supplier,
> Gatherer.Integrator.ofGreedy((state, element, _) -> {
> accumulator.accept(state, element);
> return true;
> }),
> combiner,
> (state, downstream) -> downstream.push(finisher.apply(state)));
> }
> This is eaxctly how Gatherer.fold() works.
> Is there a reason why such method does not exist ?
> regards,
> Rémi
Re: Gatherer: spliterator characteristics are not propagated
> From: "Viktor Klang"
> To: "Remi Forax" , "core-libs-dev"
>
> Sent: Wednesday, January 17, 2024 8:48:07 PM
> Subject: Re: Gatherer: spliterator characteristics are not propagated
> Hi Rémi,
> You can find some of my benches here: [
> https://github.com/openjdk/jdk/tree/master/test/micro/org/openjdk/bench/java/util/stream/ops/ref
> |
> https://github.com/openjdk/jdk/tree/master/test/micro/org/openjdk/bench/java/util/stream/ops/ref
> ]
> Initially I had Characteristics such as ORDERED etc on Gatherer but it just
> didn't end up worth it when looking at the bench results over a wide array of
> stream sizes and number of operations.
I think there are 3 gatherer characteristics that make sense: KEEP_SORTED,
KEEP_DISTINCT and KEEP_SIZED,
all of them say that if the stream was sorted/distinct/sized then the stream
returned by a call to gather() is still sorted (with the same comparator),
distinct or sized.
As examples, map() is KEEP_SIZED, filter() is KEEP_SORTED | KEEP_DISTINCT and
windowFixed is KEEP_DISTINCT.
[CC Paul, so he can correct me if i'm saying something stupid]
Now for the benchmarks, it depends what you want to measure, benchmarking
streams is tricky. This is what i know about benchmarking streams.
First, the JIT has two ways to profile types at runtime,
Either a method takes a function as parameter
void map(Function function) {
function.apply(...)
}
and when map is called with a subtype of Function, the JIT will propagate the
exact type when map is inlined,
Or a method use a field
class Op {
Function function;
void map() {
function.apply(...)
}
}
in that case, the VM records the profile of function.apply() and if there are
more than two different profiles, the VM declare profile poluttion and do not
try to optimize.
The Stream implementation tries very hard to use only parameters instead of
fields, that's why it does not use classical Iterator that are pull iterator (a
filter iterator requires a field) but a Spliterator which is a push iterator,
the element is sent as parameter of the consumer.That's also why collect does
not use the builder pattern (that accumulate values in fields) but a Collector
that publish the functions to be called as parameter.
Obvisously, this is more complex than that, a Collector stores the functions in
fields so it should not work well but the implementation uses a record that
plays well with escape analysis. Escape analysis see the fields of an instance
as parameters so the functions of a Collector are correctly propagated (if the
escape analysis works). And lambdas are using invokedynamic, and the VM tries
really hard to inline invokedynamic, so lambdas (that captures value or not)
are routinely fully inlined with the intermediate operation of a stream.
In your tests, i've not seen comparaisons between an existing method like map()
or filter() followed by a sorted()/distinct()/count()/toList(), i.e. operations
where the characteristics KEEP_* have an impact and their equivalent using a
Gatherer.
> Cheers,
> √
regards,
Rémi
> Viktor Klang
> Software Architect, Java Platform Group
> Oracle
> From: core-libs-dev on behalf of Remi Forax
>
> Sent: Wednesday, 17 January 2024 16:48
> To: core-libs-dev
> Subject: Gatherer: spliterator characteristics are not propagated
> While doing some benchmarking of the Gatherer API, i've found that the
> characteristics of the spliterator was not propagated by the method
> Stream.gather() making the stream slower than it should.
> As an example, there is no way when reimplementing map() using a Gatherer to
> say
> that this intermediate operation keep the size, which is important if the
> terminal operation is toList() because if the size is known, toList() will
> presize the List and avoid the creation of an intermediary ArrayList.
> See [
> https://github.com/forax/we_are_all_to_gather/blob/master/src/main/java/com/gihtub/forax/wearealltogather/bench/MapGathererBenchmark.java
> |
> https://github.com/forax/we_are_all_to_gather/blob/master/src/main/java/com/gihtub/forax/wearealltogather/bench/MapGathererBenchmark.java
> ]
> I think that adding a way to propagate the spliterator characteristics
> through a
> Gatherer would greatly improve the performance of commons streams (at least
> all
> the ones that end with a call to toList).
> I have some idea of how to do that, but I prefer first to hear if i've
> overlook
> something and if improving the performance is something worth changing the
> API.
> regards,
> Rémi
Re: Gatherer: spliterator characteristics are not propagated
> From: "Viktor Klang"
> To: "Remi Forax"
> Cc: "core-libs-dev" , "Paul Sandoz"
>
> Sent: Thursday, January 18, 2024 3:36:07 PM
> Subject: Re: Gatherer: spliterator characteristics are not propagated
> I suspect that it is a rather slippery slope, once KEEP-flags are added, then
> others will want to be able to have INJECT-flags, and then people might have
> different opinions w.r.t. the default should be to clear all flags etc.
> And that's even before one looks at the composition-part of it, what are the
> flags for A.andThen(B)? (then extrapolate to N compositions and the available
> set of flags always approaches 0)
> I spent quite a bit of time on this and in the end tracking all this info, and
> making sure that the flags of implementations correspond to the actual
> behavior, just ended up costing performance for most streams and introduced an
> extra dimension to creation and maintenance which I had a hard time
> justifying.
It can be a slippery slope if we were designing from the ground up but the
stream implementation already exists and SORTED, DISTINCT and SIZED are the
flags that are already tracked by the current implementation.
Currently only SHORT_CIRCUIT is set (if not greedy),
see [
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/GathererOp.java#L209
|
https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/GathererOp.java#L209
]
And for A.andThen(B), A.flags & B.flags should work, the stream is sorted if
both gatherers keep it sorted.
> Making specific, rare, combinations of operations faster at the expense of
> making 99% of all others slower is a hard pill for most to swallow.
I suppose that if those flags already exist, it's because they have a purpose
and i do not understand how it can make the other operations slower.
> Cheers,
> √
regards,
Rémi
> Viktor Klang
> Software Architect, Java Platform Group
> Oracle
> From: fo...@univ-mlv.fr
> Sent: Thursday, 18 January 2024 10:28
> To: Viktor Klang
> Cc: core-libs-dev ; Paul Sandoz
>
> Subject: [External] : Re: Gatherer: spliterator characteristics are not
> propagated
>> From: "Viktor Klang"
>> To: "Remi Forax" , "core-libs-dev"
>>
>> Sent: Wednesday, January 17, 2024 8:48:07 PM
>> Subject: Re: Gatherer: spliterator characteristics are not propagated
>> Hi Rémi,
>> You can find some of my benches here: [
>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/tree/master/test/micro/org/openjdk/bench/java/util/stream/ops/ref__;!!ACWV5N9M2RV99hQ!JJy6F9NoL6wKZQK5158up_fTRvH8X7F6JK8T7Euuf8vzbSQbr23eWa9S_yb61ksONVrLrdesCF_au5zyje2l$
>> |
>> https://github.com/openjdk/jdk/tree/master/test/micro/org/openjdk/bench/java/util/stream/ops/ref
>> ]
>> Initially I had Characteristics such as ORDERED etc on Gatherer but it just
>> didn't end up worth it when looking at the bench results over a wide array of
>> stream sizes and number of operations.
> I think there are 3 gatherer characteristics that make sense: KEEP_SORTED,
> KEEP_DISTINCT and KEEP_SIZED,
> all of them say that if the stream was sorted/distinct/sized then the stream
> returned by a call to gather() is still sorted (with the same comparator),
> distinct or sized.
> As examples, map() is KEEP_SIZED, filter() is KEEP_SORTED | KEEP_DISTINCT and
> windowFixed is KEEP_DISTINCT.
> [CC Paul, so he can correct me if i'm saying something stupid]
> Now for the benchmarks, it depends what you want to measure, benchmarking
> streams is tricky. This is what i know about benchmarking streams.
> First, the JIT has two ways to profile types at runtime,
> Either a method takes a function as parameter
> void map(Function function) {
> function.apply(...)
> }
> and when map is called with a subtype of Function, the JIT will propagate the
> exact type when map is inlined,
> Or a method use a field
> class Op {
> Function function;
> void map() {
> function.apply(...)
> }
> }
> in that case, the VM records the profile of function.apply() and if there are
> more than two different profiles, the VM declare profile poluttion and do not
> try to optimize.
> The Stream implementation tries very hard to use only parameters instead of
> fields, that's why it does not use classical Iterator that are pull iterator
> (a
> filter iterator requires a field) but a Spliterator which is a push iterator,
> the element is sent as parameter of the consumer.That's also why collect does
> not use the builder pattern (that accumulate values in fields) but a Collector
> that publish the functions to be called as parameter.
>
Re: Gatherer API : wildcards complaint
> From: "Viktor Klang"
> To: "Remi Forax"
> Cc: "core-libs-dev"
> Sent: Wednesday, January 17, 2024 9:00:32 PM
> Subject: Re: Gatherer API : wildcards complaint
> Ah, now I see what you mean! Thank you \uD83D\uDC4D
> The reason for the signature of `Gatherer.of` was to mirror as much as
> possible
> of `Collector.of`[1] so I would argue that if we tweak the variance of one
> then
> we should consider tweaking it for both.
> 1: [
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Collector.java#L264
> |
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Collector.java#L264
> ]
I agree.
In terms of code, i suppose that we will need to add some unsafe cast because
the Gatherer interface should not use wildcard (it will make the code that use
the Gatherer awkward) but the factory methods should use wildcards.
Those unsafe casts are safe because function parameter types are contravariant
and return type covariant but there is no way to express than in the Java type
system.
> Cheers,
> √
Rémi
> Viktor Klang
> Software Architect, Java Platform Group
> Oracle
> From: fo...@univ-mlv.fr
> Sent: Wednesday, 17 January 2024 20:49
> To: Viktor Klang
> Cc: core-libs-dev
> Subject: [External] : Re: Gatherer API : wildcards complaint
>> From: "Viktor Klang"
>> To: "Remi Forax" , "core-libs-dev"
>>
>> Sent: Wednesday, January 17, 2024 5:49:01 PM
>> Subject: Re: Gatherer API : wildcards complaint
>> Hi Rémi,
>> Thank you for the feedback—examples would be much appreciated!
> Here is an example with an interface and a class,
> interface Counter {
> void increment ();
> int value ();
> }
> Gatherer < String , Counter , Integer > count () {
> class CounterImpl implements Counter {
> int counter ;
> @Override
> public void increment () {
> counter ++;
> }
> @Override
> public int value () {
> return counter ;
> }
> }
> Supplier < CounterImpl > initializer = CounterImpl :: new ;
> Gatherer . Integrator < Counter , String , Gatherer . Downstream Integer >> integrator = ( counter , _ , _ ) -> {
> counter .increment();
> return true ;
> };
> BiConsumer < Counter , Gatherer . Downstream > finisher = (
> counter , downstream ) -> {
> downstream .push( counter .value());
> };
> return Gatherer . ofSequential ( initializer , integrator , finisher ); //
> does
> not compile :(
> }
> void main () {
> System . out .println( Stream . of ( "foo"
> ).gather(count()).findFirst().orElseThrow());
> }
> if instead of explicitly typing each functions, we directly call ofSequential,
> it works
> return Gatherer . ofSequential (
> CounterImpl :: new ,
> ( Counter counter , String _ , Gatherer . Downstream _ )
> -> {
> counter .increment();
> return true ;
> },
> ( Counter counter , Gatherer . Downstream downstream ) -> {
> downstream .push( counter .value());
> }
> );
> because here, CounterImpl::new is inferred as Supplier.
>> Cheers,
>> √
>> Viktor Klang
>> Software Architect, Java Platform Group
>> Oracle
>> From: core-libs-dev on behalf of Remi Forax
>>
>> Sent: Wednesday, 17 January 2024 16:55
>> To: core-libs-dev
>> Subject: Gatherer API : wildcards complaint
>> Hello,
>> this is a minor complaint but I do not see a raison to not getting this
>> right.
>> Currently the Gatherer API does not use the wildcard correctly, which is not
>> fully an issue because there is "enough" wildcards that if you rely on the
>> inference, it will work.
>> The problem is that when you write code, you make mistakes and usually when
>> you
>> have a typing issue, a way to debug it is to fix the type arguments
>> de-activating the inference.
>> But because there are some missing wildcards, this debugging strategy just
>> fail
>> flat with more typing errors.
>> I think that fixing the missing wildcards will hep users (or a least me) to
>> have
>> a better experience when using the Gatherer API.
>> I can help and propose a PR if you want.
>> regards,
>> Rémi
Re: Gatherer: spliterator characteristics are not propagated
> From: "Viktor Klang"
> To: "Remi Forax"
> Cc: "core-libs-dev" , "Paul Sandoz"
>
> Sent: Thursday, January 18, 2024 5:14:38 PM
> Subject: Re: Gatherer: spliterator characteristics are not propagated
>> And for A.andThen(B), A.flags & B.flags should work, the stream is sorted if
>> both gatherers keep it sorted.
> That is unfortunately not the case. That would presume that you can implement
> the composition such that it can preserve all the common flags. Some flags
> could be "dominant" and some "recessive" to use genetics nomenclature.
Some flags of the stream pipeline are "recessive", mainly SHORT_CIRCUIT, but
not the characteristics of the Gatherer which can have the corresponding
"dominant" flag, GREEDY, in this case.
And the same for sequential, the flag should be PARALELIZABLE and not
SEQUENTIAL.
The idea is that the Gatherer characteristics can have the same bit set at the
same position as the stream op flags (as defined by StreamOpFlag).
So KEEP_DISTINCT is in position 0, KEEP_SORTED in in position 2 and KEEP_SIZED
is in position 3.
For GREEDY, we use the same position as SHORT_CIRCUIT and we will flip the bit
(using XOR) when we want to extract the stream op flags from the
characteristics
All the factory methods call the generic of() with a combination of
PARALELIZABLE and STATELESS and the user can adds the characteristics GREEDY,
KEEP_DISTINCT, KEEP_SORTED and KEEP_SIZED (otherwise an exception should be
raised).
In StreamOpFlag, there are two unused positions (14 and 15), that's perfect for
our two new states PARALELIZABLE and STATELESS, so no problem here (technically
we can also reuse positions of the Spliterator characteristic given that those
flags are masked before being sent to the GathererOp super constructor).
The way to transform a Gatherer characteristics op to a stream flags op is to
flip the bits corresponding to SHORT_CIRCUIT, add the highter bit of all flags
but SHORT-CIRCUIT (because stream op flags are encoded using 2 bits) and mask
to only retain SHORT_CIRCUIT, KEEP_DISTINCT, KEEP_SORTED and KEEP_SIZED.
public static int toOpFlags ( int characteristics ) {
return (( characteristics ^ SHORT_CIRCUIT_MASK ) | HIGHER_BITS ) &
STREAM_OP_MASK ;
}
see below for a full script.
>> I suppose that if those flags already exist, it's because they have a purpose
>> and i do not understand how it can make the other operations slower.
> Extra invocations, extra storage, and extra composition overhead is not free.
> Since Stream is one-shot you need to include the construction cost with the
> execution cost. For something like an empty Stream construction cost scan be
> 90+% of the total costs.
If you create a Gatherer, the characteristics is a constant (so the validity
check is removed, it's just a mask and a test) so the result of calling
toOpFlags() is a constant too.
If the factory method is not inlined, the cost is 3 bitwise operations which is
I believe faster than the "instanceof Greedy" used in the current code.
> Cheers,
> √
regards,
Rémi
---
public class GathererFlag {
// cut and paste from StreamOpFlag
/**
* The bit pattern for setting/injecting a flag.
*/
private static final int SET_BITS = 0b01 ;
/**
* The bit pattern for clearing a flag.
*/
private static final int CLEAR_BITS = 0b10 ;
/**
* The bit pattern for preserving a flag.
*/
private static final int PRESERVE_BITS = 0b11 ;
private static int position ( int opFlagSet ) {
return Integer . numberOfTrailingZeros ( opFlagSet ) >> 1 ;
}
private static final int DISTINCT_POSITION = position ( StreamOpFlag .
IS_DISTINCT );
private static final int SORTED_POSITION = position ( StreamOpFlag . IS_SORTED
);
private static final int SIZED_POSITION = position ( StreamOpFlag . IS_SIZED );
private static final int SHORT_CIRCUIT_POSITION = position ( StreamOpFlag .
IS_SHORT_CIRCUIT );
private static final int STATELESS_POSITION = 14 ;
private static final int PARELLIZABLE_POSITION = 15 ;
public static final int PARELLIZABLE = SET_BITS << ( PARELLIZABLE_POSITION << 1
);
public static final int STATELESS = SET_BITS << ( STATELESS_POSITION << 1 );
public static final int GREEDY = SET_BITS << ( SHORT_CIRCUIT_POSITION << 1 );
public static final int KEEP_DISTINCT = SET_BITS << ( DISTINCT_POSITION << 1 );
public static final int KEEP_SORTED = SET_BITS << ( SORTED_POSITION << 1 );
public static final int KEEP_SIZED = SET_BITS << ( SIZED_POSITION << 1 );
private static final int SHORT_CIRCUIT_MASK = SET_BITS << (
SHORT_CIRCUIT_POSITION << 1 );
// no GREEDY here
private static final int HIGHER_BITS = ( PARELLIZABLE | STATELESS |
KEEP_DISTINCT | KEEP_SORTED | KEEP_SIZED ) <&
Re: Gatherer: spliterator characteristics are not propagated
> From: "Viktor Klang"
> To: "Remi Forax"
> Cc: "core-libs-dev" , "Paul Sandoz"
>
> Sent: Monday, January 22, 2024 4:22:11 PM
> Subject: Re: Gatherer: spliterator characteristics are not propagated
> Hi Rémi,
Hello,
> For instance, stateless is neither recessive nor dominant, since the
> composition
> of two stateless operations is only ever stateless if they both are greedy as
> well: [
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherers.java#L588
> |
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherers.java#L588
> ]
Okay, so choosing SEQUENTIAL vs PARALELLIZABLE is not that important given that
the combination is ad-hoc, reflecting the characterristics is enough.
> So even if making it represented as ints (more like Spliterator, rather than
> Collector) makes things faster, it's still both work to track, propagate, and
> also becomes a side-channel that needs to remain in sync with the actual
> implementation of the logic.
The flags are in sync with the implementation because the only way to create a
Gatherer if through the factory methods and those factory methods (and only
them) compute the proper combination of SEQUENTIAL | STATELESS | GREEDY. A user
should not be able to set those flags. Only the flags KEEP_* are settable by a
user.
> One could argue that logic such as: someCollection.stream().map(…).count() is
> a
> performance bug/inefficiency in an of itself as it would be faster to do
> someCollection.size().
The stream implementation has a whole mechanism in place to propagate/preverse
flags like SIZED, DISTINCT or SORTED. For me, discussing about the merit of
this mechanism seems a little off topic. I would prefer the Gatherer to be a
good citizen and works seemlessly with the other intermediary operations.
> Cheers,
> √
regards,
Rémi
> Viktor Klang
> Software Architect, Java Platform Group
> Oracle
> From: fo...@univ-mlv.fr
> Sent: Saturday, 20 January 2024 17:40
> To: Viktor Klang
> Cc: core-libs-dev ; Paul Sandoz
>
> Subject: [External] : Re: Gatherer: spliterator characteristics are not
> propagated
>> From: "Viktor Klang"
>> To: "Remi Forax"
>> Cc: "core-libs-dev" , "Paul Sandoz"
>>
>> Sent: Thursday, January 18, 2024 5:14:38 PM
>> Subject: Re: Gatherer: spliterator characteristics are not propagated
>>> And for A.andThen(B), A.flags & B.flags should work, the stream is sorted if
>>> both gatherers keep it sorted.
>> That is unfortunately not the case. That would presume that you can implement
>> the composition such that it can preserve all the common flags. Some flags
>> could be "dominant" and some "recessive" to use genetics nomenclature.
> Some flags of the stream pipeline are "recessive", mainly SHORT_CIRCUIT, but
> not
> the characteristics of the Gatherer which can have the corresponding
> "dominant"
> flag, GREEDY, in this case.
> And the same for sequential, the flag should be PARALELIZABLE and not
> SEQUENTIAL.
> The idea is that the Gatherer characteristics can have the same bit set at the
> same position as the stream op flags (as defined by StreamOpFlag).
> So KEEP_DISTINCT is in position 0, KEEP_SORTED in in position 2 and KEEP_SIZED
> is in position 3.
> For GREEDY, we use the same position as SHORT_CIRCUIT and we will flip the bit
> (using XOR) when we want to extract the stream op flags from the
> characteristics
> All the factory methods call the generic of() with a combination of
> PARALELIZABLE and STATELESS and the user can adds the characteristics GREEDY,
> KEEP_DISTINCT, KEEP_SORTED and KEEP_SIZED (otherwise an exception should be
> raised).
> In StreamOpFlag, there are two unused positions (14 and 15), that's perfect
> for
> our two new states PARALELIZABLE and STATELESS, so no problem here
> (technically
> we can also reuse positions of the Spliterator characteristic given that those
> flags are masked before being sent to the GathererOp super constructor).
> The way to transform a Gatherer characteristics op to a stream flags op is to
> flip the bits corresponding to SHORT_CIRCUIT, add the highter bit of all flags
> but SHORT-CIRCUIT (because stream op flags are encoded using 2 bits) and mask
> to only retain SHORT_CIRCUIT, KEEP_DISTINCT, KEEP_SORTED and KEEP_SIZED.
> public static int toOpFlags ( int characteristics ) {
> return (( characteristics ^ SHORT_CIRCUIT_MASK ) | HIGHER_BITS ) &
> STREAM_OP_MASK ;
> }
> see below for a full script.
>>> I suppose that if those flags already exist, it's
Re: Gatherer: spliterator characteristics are not propagated
> From: "Viktor Klang" > To: "Remi Forax" > Cc: "core-libs-dev" , "Paul Sandoz" > > Sent: Monday, January 22, 2024 10:06:27 PM > Subject: Re: Gatherer: spliterator characteristics are not propagated >> The flags are in sync with the implementation because the only way to create >> a >> Gatherer if through the factory methods and those factory methods (and only >> them) compute the proper combination of SEQUENTIAL | STATELESS | GREEDY. A >> user >> should not be able to set those flags. Only the flags KEEP_* are settable by >> a >> user. > But I presume this also requires to have a `int characteristics()`-method on > the > Gatherer interfacewhich means that users who are not using the factory methods > will have full possibility of not only returning the flags, but returning any > int. The current implementation suffers the same kind of issue, it's easy to write a mutable Gatherer and change the functions after creation, worst, right in the middle of a call to stream.gather(...). Perhaps the Gatherer interface should be sealed ? We did not have that option during the 1.8 timeframe, when the Collector API was created. >> The stream implementation has a whole mechanism in place to >> propagate/preverse >> flags like SIZED, DISTINCT or SORTED. For me, discussing about the merit of >> this mechanism seems a little off topic. I would prefer the Gatherer to be a >> good citizen and works seemlessly with the other intermediary operations. > I can see where you're coming from here, but to me, adding API surface needs > to > pull its weight. > In this case I wasn't convinced that it did, hence we're having this > conversation. \uD83D\uDE42 > Cheers, > √ regards, Rémi > Viktor Klang > Software Architect, Java Platform Group > Oracle > From: fo...@univ-mlv.fr > Sent: Monday, 22 January 2024 19:56 > To: Viktor Klang > Cc: core-libs-dev ; Paul Sandoz > > Subject: [External] : Re: Gatherer: spliterator characteristics are not > propagated >> From: "Viktor Klang" >> To: "Remi Forax" >> Cc: "core-libs-dev" , "Paul Sandoz" >> >> Sent: Monday, January 22, 2024 4:22:11 PM >> Subject: Re: Gatherer: spliterator characteristics are not propagated >> Hi Rémi, > Hello, >> For instance, stateless is neither recessive nor dominant, since the >> composition >> of two stateless operations is only ever stateless if they both are greedy as >> well: [ >> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherers.java*L588__;Iw!!ACWV5N9M2RV99hQ!Lm52jd6kovd5t-cmrqqSLiRcIajBGXLxh85LO3eeiL6UxbKZuNPcUnO6z2i0FzMEoNr7U-cOBuWPCjo57FVW$ >> | >> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherers.java#L588 >> ] > Okay, so choosing SEQUENTIAL vs PARALELLIZABLE is not that important given > that > the combination is ad-hoc, reflecting the characterristics is enough. >> So even if making it represented as ints (more like Spliterator, rather than >> Collector) makes things faster, it's still both work to track, propagate, and >> also becomes a side-channel that needs to remain in sync with the actual >> implementation of the logic. > The flags are in sync with the implementation because the only way to create a > Gatherer if through the factory methods and those factory methods (and only > them) compute the proper combination of SEQUENTIAL | STATELESS | GREEDY. A > user > should not be able to set those flags. Only the flags KEEP_* are settable by a > user. >> One could argue that logic such as: someCollection.stream().map(…).count() >> is a >> performance bug/inefficiency in an of itself as it would be faster to do >> someCollection.size(). > The stream implementation has a whole mechanism in place to propagate/preverse > flags like SIZED, DISTINCT or SORTED. For me, discussing about the merit of > this mechanism seems a little off topic. I would prefer the Gatherer to be a > good citizen and works seemlessly with the other intermediary operations. > Cheers, > √ > Viktor Klang > Software Architect, Java Platform Group > Oracle > From: fo...@univ-mlv.fr > Sent: Monday, 22 January 2024 19:56 > To: Viktor Klang > Cc: core-libs-dev ; Paul Sandoz > > Subject: [External] : Re: Gatherer: spliterator characteristics are not > propagated >> From: "Viktor Klang" >> To: "Remi Forax" >> Cc: "core-libs-dev" , "Paul Sandoz" >> >> Sent: Monday, January 22,
Re: Gatherer: spliterator characteristics are not propagated
> From: "Viktor Klang" > To: "Remi Forax" > Cc: "core-libs-dev" , "Paul Sandoz" > > Sent: Wednesday, January 24, 2024 2:34:11 PM > Subject: Re: Gatherer: spliterator characteristics are not propagated > Presuming that you mean mutating the Gatherer such that its behavior isn't > stable, the difference (at least to me) is that creating such a mutable > Gatherer would violate the specification of Gatherer: [ > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherer.java#L63 > | > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherer.java#L63 > ] If there is a method characteristics, its spec will also indicates that a flag like STATELESS is only allowed but be used if the state is null. Not using STATELESS correctly will be also a violation of the spec. > Cheers, > √ regards, Rémi > Viktor Klang > Software Architect, Java Platform Group > Oracle > From: fo...@univ-mlv.fr > Sent: Tuesday, 23 January 2024 21:04 > To: Viktor Klang > Cc: core-libs-dev ; Paul Sandoz > > Subject: [External] : Re: Gatherer: spliterator characteristics are not > propagated >> From: "Viktor Klang" >> To: "Remi Forax" >> Cc: "core-libs-dev" , "Paul Sandoz" >> >> Sent: Monday, January 22, 2024 10:06:27 PM >> Subject: Re: Gatherer: spliterator characteristics are not propagated >>> The flags are in sync with the implementation because the only way to >>> create a >>> Gatherer if through the factory methods and those factory methods (and only >>> them) compute the proper combination of SEQUENTIAL | STATELESS | GREEDY. A >>> user >>> should not be able to set those flags. Only the flags KEEP_* are settable >>> by a >>> user. >> But I presume this also requires to have a `int characteristics()`-method on >> the >> Gatherer interfacewhich means that users who are not using the factory >> methods >> will have full possibility of not only returning the flags, but returning any >> int. > The current implementation suffers the same kind of issue, it's easy to write > a > mutable Gatherer and change the functions after creation, worst, right in the > middle of a call to stream.gather(...). > Perhaps the Gatherer interface should be sealed ? We did not have that option > during the 1.8 timeframe, when the Collector API was created. >>> The stream implementation has a whole mechanism in place to >>> propagate/preverse >>> flags like SIZED, DISTINCT or SORTED. For me, discussing about the merit of >>> this mechanism seems a little off topic. I would prefer the Gatherer to be a >>> good citizen and works seemlessly with the other intermediary operations. >> I can see where you're coming from here, but to me, adding API surface needs >> to >> pull its weight. >> In this case I wasn't convinced that it did, hence we're having this >> conversation. \uD83D\uDE42 >> Cheers, >> √ > regards, > Rémi >> Viktor Klang >> Software Architect, Java Platform Group >> Oracle >> From: fo...@univ-mlv.fr >> Sent: Monday, 22 January 2024 19:56 >> To: Viktor Klang >> Cc: core-libs-dev ; Paul Sandoz >> >> Subject: [External] : Re: Gatherer: spliterator characteristics are not >> propagated >>> From: "Viktor Klang" >>> To: "Remi Forax" >>> Cc: "core-libs-dev" , "Paul Sandoz" >>> >>> Sent: Monday, January 22, 2024 4:22:11 PM >>> Subject: Re: Gatherer: spliterator characteristics are not propagated >>> Hi Rémi, >> Hello, >>> For instance, stateless is neither recessive nor dominant, since the >>> composition >>> of two stateless operations is only ever stateless if they both are greedy >>> as >>> well: [ >>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherers.java*L588__;Iw!!ACWV5N9M2RV99hQ!Lm52jd6kovd5t-cmrqqSLiRcIajBGXLxh85LO3eeiL6UxbKZuNPcUnO6z2i0FzMEoNr7U-cOBuWPCjo57FVW$ >>> | >>> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherers.java#L588 >>> ] >> Okay, so choosing SEQUENTIAL vs PARALELLIZABLE is not that important given >> that >> the combination is ad-hoc, reflecting the characterristics is enough. >>> So even if making it represented as ints (more like Spliterator, rather than >>> Collector) makes thin
Re: Gatherer: spliterator characteristics are not propagated
> From: "Viktor Klang" > To: "Remi Forax" > Cc: "core-libs-dev" , "Paul Sandoz" > > Sent: Wednesday, January 24, 2024 2:45:15 PM > Subject: Re: Gatherer: spliterator characteristics are not propagated > As a (related) side-note, the ability to implement the interface directly has > a > significant benefit to being able to have tighter control on > efficiency/performance as well as behavior under composition, here are some > examples: [ > https://github.com/openjdk/jdk/blob/master/test/micro/org/openjdk/bench/java/util/stream/ops/ref/BenchmarkGathererImpls.java#L113 > | > https://github.com/openjdk/jdk/blob/master/test/micro/org/openjdk/bench/java/util/stream/ops/ref/BenchmarkGathererImpls.java#L113 > ] I've not seen any differences in term of performance: MapGathererBenchmark.gatherer_map_sum avgt 5 546.058 ± 4.224 us/op MapGathererBenchmark.gatherer_mapsublcass_sum avgt 5 546.391 ± 1.508 us/op but yes, being able to override andThen() is important. > Cheers, > √ Rémi > Viktor Klang > Software Architect, Java Platform Group > Oracle > From: core-libs-dev on behalf of Viktor Klang > > Sent: Wednesday, 24 January 2024 14:34 > To: fo...@univ-mlv.fr > Cc: core-libs-dev ; Paul Sandoz > > Subject: Re: Gatherer: spliterator characteristics are not propagated > Presuming that you mean mutating the Gatherer such that its behavior isn't > stable, the difference (at least to me) is that creating such a mutable > Gatherer would violate the specification of Gatherer: [ > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherer.java#L63 > | > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/stream/Gatherer.java#L63 > ] > Cheers, > √ > Viktor Klang > Software Architect, Java Platform Group > Oracle > From: fo...@univ-mlv.fr > Sent: Tuesday, 23 January 2024 21:04 > To: Viktor Klang > Cc: core-libs-dev ; Paul Sandoz > > Subject: [External] : Re: Gatherer: spliterator characteristics are not > propagated >> From: "Viktor Klang" >> To: "Remi Forax" >> Cc: "core-libs-dev" , "Paul Sandoz" >> >> Sent: Monday, January 22, 2024 10:06:27 PM >> Subject: Re: Gatherer: spliterator characteristics are not propagated >>> The flags are in sync with the implementation because the only way to >>> create a >>> Gatherer if through the factory methods and those factory methods (and only >>> them) compute the proper combination of SEQUENTIAL | STATELESS | GREEDY. A >>> user >>> should not be able to set those flags. Only the flags KEEP_* are settable >>> by a >>> user. >> But I presume this also requires to have a `int characteristics()`-method on >> the >> Gatherer interfacewhich means that users who are not using the factory >> methods >> will have full possibility of not only returning the flags, but returning any >> int. > The current implementation suffers the same kind of issue, it's easy to write > a > mutable Gatherer and change the functions after creation, worst, right in the > middle of a call to stream.gather(...). > Perhaps the Gatherer interface should be sealed ? We did not have that option > during the 1.8 timeframe, when the Collector API was created. >>> The stream implementation has a whole mechanism in place to >>> propagate/preverse >>> flags like SIZED, DISTINCT or SORTED. For me, discussing about the merit of >>> this mechanism seems a little off topic. I would prefer the Gatherer to be a >>> good citizen and works seemlessly with the other intermediary operations. >> I can see where you're coming from here, but to me, adding API surface needs >> to >> pull its weight. >> In this case I wasn't convinced that it did, hence we're having this >> conversation. \uD83D\uDE42 >> Cheers, >> √ > regards, > Rémi >> Viktor Klang >> Software Architect, Java Platform Group >> Oracle >> From: fo...@univ-mlv.fr >> Sent: Monday, 22 January 2024 19:56 >> To: Viktor Klang >> Cc: core-libs-dev ; Paul Sandoz >> >> Subject: [External] : Re: Gatherer: spliterator characteristics are not >> propagated >>> From: "Viktor Klang" >>> To: "Remi Forax" >>> Cc: "core-libs-dev" , "Paul Sandoz" >>> >>> Sent: Monday, January 22, 2024 4:22:11 PM >>> Subject: Re: Gatherer: spliterator characteristics are not propagated >>> Hi Rémi, >> Hello,
Re: Addition of Predicate-based findIndex and findLastIndex methods to java.util.List
> From: "David Alayachew"
> To: "Remi Forax"
> Cc: "ІП-24 Олександр Ротань" , "core-libs-dev"
>
> Sent: Friday, April 19, 2024 11:02:12 PM
> Subject: Re: Addition of Predicate-based findIndex and findLastIndex methods
> to
> java.util.List
> No Rémi, I don't think your idea is the right approach. You are working on the
> wrong level of abstraction.
Hello David,
> Many users ask requests like this all the time, and what you are suggesting
> would be even more error-prone than the equivalent for loop or the IntStream
> suggestion that the user above requested.
Could you give an example of why it is more error prone ?
> Not to mention that getting it to parallelize would be a task many users are
> likely to mess up -- either in correctness or in performance.
Correctness is not an issue athat's why i've used ofSequential() when creating
the Gatherer.
You are right that performance can be an issue, the Gatherer API is more
limited compared to what a Spliterator can do.
> I think you would get far more mileage from adding 2 methods on the list
> interface streamWithIndex() and parallelStreamWithIndex() that would return a
> Stream>, as opposed to just Stream.
Any stream can be "indexed", having an index is not an intrisinc property of a
stream of List, so adding an index is more an intermediate operation than a
source operation.
So more like
list.stream().withIndex()
than like
list.streamWithIndex()
Now, we (the lambda-util expert group) talked 10 years ago to add such design
but decide to postpone it because WithIndex should be a value type and the
Stream API should be specialized to avoid unecessary allocations.
So you are right that withIndex() is a more general design but because of the
performance issue we can not yet offer such design.
As a counter argument, you can say that we have the same issue with the
Collector API and the Gatherer API which are both not specialized but at least
for a Collector, the fact that the accumulator does a side effect which is
usualyl a write in memory is slow anyway. And this is also true for a Gatherer,
it works well when the Gatherer is itself followed by a Collector.
> That way, users are not writing a custom Gatherer each time they want to work
> with the index. They just have the index be a field in the object. They can
> work with it the same way they would any other object field.
This custom Gatherer can be declared in Gatherers (with an 's' at the end).
> Furthermore, doing it this way makes the correct answer obvious. If I need to
> do
> something with an index, stream with the index.
Technically, here, you want the result to be an index, so you may not want to
pay the price of creating an index (your WithIndex objetcs) for every elements
but just one when the predicate is true.
This point is moot if the WithIndex is a value type and Stream is a specialized
generics but as I said above, where are not yet at that point.
> On top of that, it significantly enhances readability by making it clear to
> the
> reader that, whatever this stream is doing will require use of the index, so
> watch out for that.
With your proposal, you still need to write something like
list.withIndexStream().filter(withIndex ->
predicate(withIndex.element())).mapToInt(WithIndex::index).findFirst().orElse(-1)
I do not see the enhancement in readability.
regards,
Rémi
> On Fri, Apr 19, 2024, 1:47 PM Remi Forax < [ mailto:fo...@univ-mlv.fr |
> fo...@univ-mlv.fr ] > wrote:
>> Hello,
>> for me, it seems what you want is Collector on Stream which is able to
>> short-circuit,
>> so you can write
>> list.stream().collect(Collectors.findFirst(s -> s.contains("o")))
>> and in reverse
>> list.reversed().stream().collect(Collectors.findFirst(s -> s.contains("o")))
>> Using a Stream here is more general and will work with other collections
>> like a
>> LinkedHashSet for example.
>> Sadly, there is no way to define a short-circuiting collector :(
>> You can have a short-circuiting Gatherer like this
>> Gatherer findIndex(Predicate predicate) {
>> return Gatherer.ofSequential(
>> () -> new Object() { int index; },
>> Integrtor.ofGreedy((state, element, downstream) -> {
>> var index = state.index++;
>> if (predicate.test(element)) {
>> return downstream.push(index);
>> }
>> return true;
>> }));
>> }
>> and use it like this:
>> list.stream().gather(findIndex(s -> s.contains("o"))).findFirst().orElse(-1);
>> But it's more verbose.
>> I wonder if at the same time that the Gatherer API is introduced, the
>> Collector
>> API should
Re: Factory methods for SequencedSet and SequencedMap
> From: "Rafael Winterhalter"
> To: "Remi Forax"
> Cc: "core-libs-dev" , "joe darcy"
>
> Sent: Tuesday, January 21, 2025 10:17:35 AM
> Subject: Re: Factory methods for SequencedSet and SequencedMap
> Wouldn't this already be possible with today's union types?
> static & SequencedSet> S of(E... elements) { ... }
No, because when you have a type variable declared on a method, it means
\forall,
so here, the method of() has to work for all implementations of S, so all
implementations of a SeqenceSet.
To see the issue, you can fix the type argument, by example
TreeSet set = Set.>of("foo")
is obviously wrong but allowed by the declaration.
> Then again, I do not think that the regular Set and Map implementations should
> be sequenced, mainly to avoid that tests suffer from this sequencing.
I do not disagree :)
Rémi
> Am So., 19. Jan. 2025 um 16:18 Uhr schrieb Remi Forax < [
> mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ] >:
>> What can be done is to have Set.of()/Map.of() to delegate to
>> SequenceSet.of()/SequenceMap.of() so there is only one implementation at
>> runtime.
>> Also, technically, there is a way to change the return type in a binary
>> compatible way ... if union types are supported in the language.
>> In that case, Set.of() can be declared like this
>> static Set | SequenceSet of(E... ) {
>> ...
>> }
>> because of erasure, the binary descriptor will use Set, but the Signature
>> will
>> be Set | SequenceSet so the compiler will see the return type as a
>> SequencedSet .
>> Obviously, union types is a far bigger features than adding
>> SequenceSet/SequenceMap so it's a big if, but it may happen in the future.
>> Rémi
>>> From: "joe darcy" < [ mailto:joe.da...@oracle.com | joe.da...@oracle.com ] >
>>> To: "core-libs-dev" < [ mailto:core-libs-dev@openjdk.org |
>>> core-libs-dev@openjdk.org ] >
>>> Sent: Friday, January 17, 2025 6:30:40 PM
>>> Subject: Re: Factory methods for SequencedSet and SequencedMap
>>> On 1/16/2025 11:26 PM, Rafael Winterhalter wrote:
>>>> Would it even be possible to change the return types of Set.of(...) and
>>>> Map.of(...) without breaking binary compatibility?
>>> In short, no.
>> [...]
>>> -Joe
>> Rémi
Re: [External] : Gatherers.fold()
> From: "Viktor Klang"
> To: "Remi Forax" , "core-libs-dev"
>
> Sent: Thursday, January 2, 2025 12:26:08 PM
> Subject: Re: [External] : Gatherers.fold()
> Hi Rémi,
Happy new year,
>>Thinking a little more, i do not understand why fold takes a Supplier as first
>>parameter and not just a value given that the Gatherer is created with
> >ofSequential().
> If it didn't take a supplier then the resulting Gatherer could never be reused
> (including composition). I.e. The use of Supplier for the state type is not
> primarily/only about parallelizability, but rather reuse (which subsumes
> parallelizability).
yes, it makes sense.
Okay, so there is just a missing wildcard when the Supplier is declared in
fold, thanks to the erasure (it's not something you often write :) ), it's a
backward compatible change.
> Cheers,
> √
regards,
Rémi
> Viktor Klang
> Software Architect, Java Platform Group
> Oracle
> From: Remi Forax
> Sent: Friday, 20 December 2024 10:39
> To: core-libs-dev
> Cc: Viktor Klang
> Subject: [External] : Gatherers.fold()
> Hello Victor,
> for the Advent of code of yesterday [1], i've this code golfing solution (the
> full code is here [2])
> var input = ...;
> class Node {
> final HashMap map = new HashMap<>();
> boolean end;
> }
> var root = new Node();
> Arrays.stream(input.substring(0, input.indexOf('\n')).split(", "))
> .forEach(w -> w.chars().boxed().gather(Gatherers.fold(() -> root,
> (n, c) -> n.map.computeIfAbsent(c, _ -> new
> Node(.findFirst().orElseThrow().end = true);
> println(input.lines().skip(2).filter(w ->
> w.chars().boxed().>gather(Gatherers.fold(() -> new
> HashSet<>(Set.of(root)),
> (ms, c) -> ms.stream().flatMap(m -> Stream.ofNullable(m.map.get(c)))
> .flatMap(n -> n != null && n.end ? Stream.of(n, root) :
> Stream.ofNullable(n)).collect(Collectors.toSet(
> .findFirst().orElseThrow().stream().anyMatch(n -> n.end)).count());
> As you can see, the second call to Gatherers.fold() requires to specify the
> type
> argument, the > in front because the supplier is not declared as a
> Supplier
> Thinking a little more, i do not understand why fold takes a Supplier as first
> parameter and not just a value given that the Gatherer is created with
> ofSequential().
> regards,
> Rémi
> [1] [
> https://urldefense.com/v3/__https://adventofcode.com/2024/day/19__;!!ACWV5N9M2RV99hQ!J6APK3GQDbT-T1edAyWssnGHideZ-3rnwBQ39CsaHnc0y6OLh0fpSub1gPWjXsfzHl4OfAtxfQvA1k3dUfis$
> |
> https://urldefense.com/v3/__https://adventofcode.com/2024/day/19__;!!ACWV5N9M2RV99hQ!J6APK3GQDbT-T1edAyWssnGHideZ-3rnwBQ39CsaHnc0y6OLh0fpSub1gPWjXsfzHl4OfAtxfQvA1k3dUfis$
> ]
> [2] [
> https://urldefense.com/v3/__https://github.com/forax/advent-of-code-2024__;!!ACWV5N9M2RV99hQ!J6APK3GQDbT-T1edAyWssnGHideZ-3rnwBQ39CsaHnc0y6OLh0fpSub1gPWjXsfzHl4OfAtxfQvA1n-w8kEG$
> |
> https://urldefense.com/v3/__https://github.com/forax/advent-of-code-2024__;!!ACWV5N9M2RV99hQ!J6APK3GQDbT-T1edAyWssnGHideZ-3rnwBQ39CsaHnc0y6OLh0fpSub1gPWjXsfzHl4OfAtxfQvA1n-w8kEG$
> ]
Re: java.lang.String hashCode and @Stable ?
> From: "Glavo" > To: "Chen Liang" > Cc: "Remi Forax" , "core-libs-dev" > > Sent: Thursday, April 10, 2025 8:54:57 PM > Subject: Re: java.lang.String hashCode and @Stable ? > Hi Chen, > I think we can choose an arbitrary non-zero number and assign it to the `hash` > field when the calculated hash is 0. Usually yes, for String no. String.hashCode() computation is fully specified in the javadoc so changing the returned value is not a backward compatible change. When you compile a switch(String) the compiler computes the hashCode of the cases and switch on those values, so those hashCode values are inserted in the bytecode. If you change the algorithm of String.hashCode(), the runtime value and the compiled value will not be the same. regards, Rémi > Glavo > On Fri, Apr 11, 2025 at 2:16 AM Chen Liang < [ mailto:chen.l.li...@oracle.com > | > chen.l.li...@oracle.com ] > wrote: >> Hi Remi, >> I think this is probably due to these fields being added too early - the >> stable >> on string byte array is also added lately. >> That said, I don't think adding stable on both fields completely resolves the >> constant folding issues around string hash code. The current code can only >> constant fold non-zero hash; a zero hash is folded to a read to hash field, >> which cannot fold further because it's a read of the default value from a >> stable field. >> A solution may be to change hashIsZero to a byte field indicating 3 states - >> hash unset (0), hash computed to field, hash computed and is zero. This >> allows >> the zero hash to constant fold as well at the cost of non-constant hash >> access >> performance, as now there are two reads. >> Also this reminds me of [ https://bugs.openjdk.org/browse/JDK-8332249 | >> https://bugs.openjdk.org/browse/JDK-8332249 ] - maybe Method::hashCode was >> hot >> because the string hash code could not fold. >> Regards, Chen >> From: core-libs-dev < [ mailto:core-libs-dev-r...@openjdk.org | >> core-libs-dev-r...@openjdk.org ] > on behalf of Remi Forax < [ >> mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ] > >> Sent: Thursday, April 10, 2025 4:18 AM >> To: core-libs-dev < [ mailto:core-libs-...@openjdk.java.net | >> core-libs-...@openjdk.java.net ] > >> Subject: java.lang.String hashCode and @Stable ? >> Question, >> why String.hash and String.hashIsZero are not declared @Stable ? >> regards, >> Rémi
Re: java.lang.String hashCode and @Stable ?
> From: "Chen Liang" > To: "Remi Forax" , "core-libs-dev" > > Sent: Thursday, April 10, 2025 8:16:39 PM > Subject: Re: java.lang.String hashCode and @Stable ? > Hi Remi, > I think this is probably due to these fields being added too early - the > stable > on string byte array is also added lately. > That said, I don't think adding stable on both fields completely resolves the > constant folding issues around string hash code. The current code can only > constant fold non-zero hash; a zero hash is folded to a read to hash field, > which cannot fold further because it's a read of the default value from a > stable field. > A solution may be to change hashIsZero to a byte field indicating 3 states - > hash unset (0), hash computed to field, hash computed and is zero. This allows > the zero hash to constant fold as well at the cost of non-constant hash access > performance, as now there are two reads. yes, and obviously, it has to be done without introducing a perf regression if the String is not constant folded. I think it would be nice to try that as a follow-up of JDK-8354300. > Also this reminds me of [ https://bugs.openjdk.org/browse/JDK-8332249 | > https://bugs.openjdk.org/browse/JDK-8332249 ] - maybe Method::hashCode was hot > because the string hash code could not fold. Usually in framework like Spring, Method are cached either by the framework or by java.lang.Class own cache so the Method itself is rarely a constant. > Regards, Chen regards, Rémi > From: core-libs-dev on behalf of Remi Forax > > Sent: Thursday, April 10, 2025 4:18 AM > To: core-libs-dev > Subject: java.lang.String hashCode and @Stable ? > Question, > why String.hash and String.hashIsZero are not declared @Stable ? > regards, > Rémi
Re: [External] : Re: My experience using Java to do CLI Scripting
- Original Message -
> From: "Ron Pressler"
> To: "Remi Forax"
> Cc: "cay horstmann" , "core-libs-dev"
> , "David Alayachew"
>
> Sent: Monday, April 14, 2025 11:46:25 PM
> Subject: Re: [External] : Re: My experience using Java to do CLI Scripting
>> On 14 Apr 2025, at 21:48, Remi Forax wrote:
>>
>>
>> Hi Ron,
>> i think you need to close the inputReader
>>
>> public static String output(String... args) {
>>ProcessBuilder processBuilder = new ProcessBuilder(args);
>>try {
>> Process process = processBuilder.start();
>> try (BufferedReader reader = process.inputReader()) {
>>return reader.lines().collect(Collectors.joining("\n"));
>> }
>>} catch (IOException e) {
>> throw new IOError(e);
>>}
>> }
>
>
> When the process terminates, the InputStream underlying the pipe is closed
> (after being drained and the unread bytes served by a ByteArrayInputStream),
> which *I think* should be sufficient, but I could be wrong.
I more worried about the iterator used by lines() throwing an exception (IO or
charset related) or joining() throwing an OOME because in that case the reader
may not be closed.
>
> — Ron
regards,
Rémi
Re: My experience using Java to do CLI Scripting
- Original Message -
> From: "Stuart Marks"
> To: "Remi Forax" , "Ron Pressler"
> , "David Alayachew"
>
> Cc: "cay horstmann" , "core-libs-dev"
>
> Sent: Tuesday, April 15, 2025 12:10:54 AM
> Subject: Re: My experience using Java to do CLI Scripting
> On 4/14/25 1:48 PM, Remi Forax wrote:
>>> From: "Ron Pressler"
>>> This does what you want (and could even be combined to a single expression):
>>>
>>> Process p = new ProcessBuilder("ls", "-al").start();
>>> String result =
>>> p.inputReader().lines().collect(Collectors.joining("\n"));
>>>
>>> and it’s even nicer in the cases where you may want to process the output
>>> line
>>> by line, as many scripts do.
>>
>> Hi Ron,
>> i think you need to close the inputReader
>
> A few points here...
>
> It might be possible to get away without any Process-related cleanup. For one,
> there's no "close" method on Process or ProcessBuilder. The destroy() method
> will
> certainly clean up, but you don't want that; and waitFor() merely waits for
> termination but doesn't actually clean anything up.
>
> At least in the Unix ProcessImpl class, there's a bunch of infrastructure to
> handle
> exit of the underlying process, drain input from its stdout/stderr, and close
> the
> pipes. (I didn't look on Windows.) So setting aside termination, IOException,
> and
> what to do with stderr, it seems possible to just get the inputReader() and do
> something with the characters it emits.
You mean by making the Process Closeable ?
>
> Getting a stream of lines with lines() seems like a reasonable thing to do if
> you
> want to process the output line by line.
with the caveat that you have to close() the returned stream (like
Files.lines()), something people will often forget.
>
> There are other possibilities, taking a nod from Files, which has methods
> readAllLines() and readString(). Putting similar methods on some class in the
> Reader
> family might help considerably here. Or you could call
> Reader.transferTo(Writer)
> that will send the characters to any Writer that might have a useful
> destination.
+1 for readAllInputLines() and readInputString() on Process.
>
> s'marks
regards,
Rémi
>
>
>
>
>
>>
>>public static String output(String... args) {
>> ProcessBuilder processBuilder = new ProcessBuilder(args);
>> try {
>>Process process = processBuilder.start();
>>try (BufferedReader reader = process.inputReader()) {
>> return reader.lines().collect(Collectors.joining("\n"));
>>}
>> } catch (IOException e) {
>>throw new IOError(e);
>> }
>>}
>>
>>
>>>
>>> — Ron
>>>
>>
>> regards,
>> Rémi
>>
>>>
>>>
>>>> On 14 Apr 2025, at 20:06, Cay Horstmann wrote:
>>>>
>>>> Absolutely, ProcessBuilder/Process is the right approach.
>>>>
>>>> I realize that all those bells and whistles in the Process API are there
>>>> for a
>>>> reason, but the API is a bit clunky for the happy day path that one
>>>> usually has
>>>> in a script: running a process until it terminates and getting its output.
>>>> It
>>>> is trivial to write a couple of helper methods, but it might be nice if the
>>>> Process API could help out. Something like
>>>>
>>>> Process p = Process.waitFor("ls", "-al");
>>>> String result = p.output();
>>>>
>>>> Cheers,
>>>>
>>>> Cay
>>>>
>>>> PS. This isn't pretty in Python either:
>>>> https://docs.python.org/3/library/subprocess.html#subprocess.run
>>>>
>>>>
>>>> Il 12/04/25 17:02, Ron Pressler ha scritto:
>>>>> Hi.
>>>>> Let’s focus on ProcessBuilder and Process (as I think that’s where you
>>>>> want to
>>>>> focus, and why I think this discussion is more appropriate for
>>>>> core-libs-dev).
>>>>> Can you try to show more concretely what the pain point is in your actual
>>>>> code?
>>>>> The ProcessBuilder example is long because it does multiple things, each
>>>>> of
>>>>> which may or may not be relevant to your use case. That doing five
>
Re: [External] : Re: JDK-8072840: Presizing for Stream Collectors
> From: "Viktor Klang" > To: "Remi Forax" , "Fabian Meumertzheim" > > Cc: "Paul Sandoz" , "core-libs-dev" > > Sent: Wednesday, February 19, 2025 11:43:33 AM > Subject: Re: [External] : Re: JDK-8072840: Presizing for Stream Collectors > Might be possible to extract the "descriptive" accessors to a superinterface > of > Spliterator. > And Gatherer could either transform an incoming such instance to an estimated > outbound instance, plus an overload of initializer , and Collector would > likely > only need the supplier overload. yes, something like that. > Cheers, > √ regards, Rémi > Viktor Klang > Software Architect, Java Platform Group > Oracle > From: Remi Forax > Sent: Saturday, 15 February 2025 15:44 > To: Viktor Klang ; Fabian Meumertzheim > > Cc: Paul Sandoz ; core-libs-dev > > Subject: [External] : Re: JDK-8072840: Presizing for Stream Collectors >> From: "Viktor Klang" >> To: "Paul Sandoz" , "Fabian Meumertzheim" >> >> Cc: "core-libs-dev" >> Sent: Thursday, February 13, 2025 11:30:59 PM >> Subject: Re: JDK-8072840: Presizing for Stream Collectors >> Indeed. I hope I didn't sound discouraging about the possibility to propagate >> the stream size information. >> I merely want to emphasize that it may necessitate a slightly broader take on >> the problem of propagation of stream-instance metadata, especially in the >> face >> of Gatherers becoming a finalized feature. > We already have an abstraction for propagating metadata, it's the query part > of > Spliterator (characteristics/estimateSize/comparator etc, technically all > abstract methods that does not starts with "try"). > For a Gatherer, we need a way to say if a characteristics is preserved or > removed. > For a collector, we need a way to have a supplier that takes a Spliterator (a > synthetic one, not the one that powers the actual stream) so the > characteristics can be queried. >> It's great that you started this conversation, Fabian! >> Cheers, >> √ > regards, > Rémi >> Viktor Klang >> Software Architect, Java Platform Group >> Oracle >> From: core-libs-dev on behalf of Paul Sandoz >> >> Sent: Thursday, 13 February 2025 20:18 >> To: Fabian Meumertzheim >> Cc: core-libs-dev >> Subject: Re: JDK-8072840: Presizing for Stream Collectors >> Hi Fabian, >> Thanks for sharing and reaching out with the idea before getting too >> beholden to >> it. >> I logged this is quite a while ago. It seemed like a possible good idea at >> the >> time, although I never liked the duplication of suppliers. I have become less >> enthusiastic overtime, especially so as Gatherers have been added. (Gatherer >> is >> the underlying primitive we could not find when we were furiously developing >> streams and meeting the Java 8 deadline.) My sense is if we are going to >> address we need to think more broadly about Gatherers. And, Viktor being the >> lead on Gatherers has a good take on where this might head. >> Paul. >> > On Feb 12, 2025, at 2:09 AM, Fabian Meumertzheim >> > wrote: >> > As an avid user of Guava's ImmutableCollections, I have been >> > interested in ways to close the efficiency gap between the built-in >> > `Stream#toList()` and third-party `Collector` implementations such as >> > `ImmutableList#toImmutableList()`. I've found the biggest problem to >> > be the lack of sizing information in `Collector`s, which led to me to >> > draft a solution to JDK-8072840: >>> [ >>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/23461__;!!ACWV5N9M2RV99hQ!LTEREzHdc6ygpw-3ySfNxSnGgAE_lEgi-NVIohRizUbXqPEDTxxWv25DQMlv_BO37jHevX51iL7Jtzd7YAOB$ >> > | >> https://github.com/openjdk/jdk/pull/23461 ] >> > The benchmark shows pretty significant gains for sized streams that >> > mostly reshape data (e.g. slice records or turn a list into a map by >> > associating keys), which I've found to be a pretty common use case. >> > Before I formally send out the PR for review, I would like to gather >> > feedback on the design aspects of it (rather than the exact >> > implementation). I will thus leave it in draft mode for now, but >> > invite anyone to comment on it or on this thread. >> > Fabian
Re: Unnecessary logic is added to removeFirst and removeLast of ArrayList.
> From: "Chen Liang"
> To: "Remi Forax"
> Cc: "core-libs-dev"
> Sent: Sunday, May 11, 2025 5:41:25 PM
> Subject: Re: Unnecessary logic is added to removeFirst and removeLast of
> ArrayList.
> > where the generated assembly code by the JITs is checked by humans
> Is it?
At least, it was true during the Java 8 timeframe :)
At that time, i was involved in the effort of updating the collection API to
use lambdas.
I suppose that the introduction of value types will trigger a similar effort
(even if it seems less easier to use value types to optimize the collection API
because the spec allows a lot of modifications).
> I have heard anecdotes that it is still slower than direct array usage,
> presumably due to the modCount tracking mechanism. Don't know if escape
> analysis and the resulting stack allocation in the compiled code eliminates
> that field.
I've recently bench array vs arraylist (while testing the performance of
StableValue.list()),
for 1024 string ("0", "1", "2" etc), on my Mac M2 air, i get
// BenchmarkMode Cnt Score Error Units
// StableListBench.arrayavgt5 427,889 ± 1,553 ns/op
// StableListBench.arraylistavgt5 605,542 ± 1,372 ns/op
// StableListBench.asList avgt5 572,532 ± 2,574 ns/op
// StableListBench.list_of avgt5 615,069 ± 0,341 ns/op
// StableListBench.stable_list avgt5 1897,948 ± 2,119 ns/op
with
@Benchmark
public int list_of () {
var sum = 0 ;
for ( var item : listof ) {
sum += item .length();
}
return sum ;
}
So at least, I can say that on my mac, direct array usage is more efficient.
(BTW, using an arraylist is more effcient than using List.of()/List.copyOf()
because ListN inherits from the iterator of AbstractList instead of having its
own).
regards,
Rémi
> From: core-libs-dev on behalf of Remi Forax
>
> Sent: Sunday, May 11, 2025 5:16 AM
> To: 임민수
> Cc: core-libs-dev
> Subject: Re: Unnecessary logic is added to removeFirst and removeLast of
> ArrayList.
> Duplicating the logic is bad in some cases and not that bad in other cases.
> Here ArrayList is one of the most used class in Java (with String and
> HashMap),
> it is heavily optimized (to the point where the generated assembly code by the
> JITs is checked by humans), so duplicating the logic is not a bad idea,
> From a documentation POV, you do not have to jump through many methods if you
> want to see the implementation and in terms of performance, you want the
> inlining to be done by hand because the methods of ArrayList are usually deep
> in the call graph, so you may reach the inlining threshold right inside
> removeFirst().
> In general, you do not optimize applications and libraries the same way.
> Usually, you do not optimize application code that often but for a library,
> if a
> lot of people are using it, it may worth to spend time to optimize for the
> benefit of everybody.
> regards,
> Rémi
Re: Towards a JSON API for the JDK
- Original Message -
> From: "naoto sato"
> To: "Remi Forax" , "Paul Sandoz"
> Cc: "core-libs-dev"
> Sent: Friday, May 16, 2025 6:35:50 PM
> Subject: Re: Towards a JSON API for the JDK
> Hi Rémi,
>
> On 5/15/25 2:27 PM, Remi Forax wrote:
>> Hi Paul,
>> yes, not having a simple JSON API in Java is an issue for beginners.
>>
>> It's not clear to me why JsonArray (for example) has to be an interface
>> instead
>> of a record ?
>>
>> I understand why Json.parse() only works on String and char[] but the API
>> make
>> it too easy to have many performance issues.
>> I think you need versions using a Reader and a Path.
>> Bonus point, if there is a method walk() that also returns a JsonValue but
>> the
>> List/Map inside JsonArray/JsonObject are populated lazily.
>>
>> Minor point: Json.toDisplayString() should takes a second parameters
>> indicating
>> the number of spaces used for the indentation (like JSON.stringify in JS).
>
> That would be better. Will implement it soon.
Thank you !
>
> Naoto
Rémi
>
>>
>> regards,
>> Rémi
>>
>> - Original Message -
>>> From: "Paul Sandoz"
>>> To: "core-libs-dev"
>>> Sent: Thursday, May 15, 2025 10:30:42 PM
>>> Subject: Towards a JSON API for the JDK
>>
>>> Hi,
>>>
>>> We would like to share with you our thoughts and plans towards a JSON API
>>> for
>>> the JDK.
>>> Please see the document below.
>>>
>>> -
>>>
>>> We have had the pleasure of using a clone of this API in some experiments
>>> we are
>>> conducting with
>>> ONNX and code reflection [1]. Using the API we were able to quickly write
>>> code
>>> to ingest and convert
>>> a JSON document representing ONNX operation schema into instances of records
>>> modeling the schema
>>> (see here [2]).
>>>
>>> The overall out-of-box experience with such a minimal "batteries included”
>>> API
>>> has so far been positive.
>>>
>>> Thanks,
>>> Paul.
>>>
>>> [1] https://openjdk.org/projects/babylon/
>>> [2]
>>> https://github.com/openjdk/babylon/blob/code-reflection/cr-examples/onnx/opgen/src/main/java/oracle/code/onnx/opgen/OpSchemaParser.java#L87
>>>
>>> # Towards a JSON API for the JDK
>>>
>>> One of the most common requests for the JDK is an API for parsing and
>>> generating
>>> JSON. While JSON originated as a text-based serialization format for JSON
>>> objects ("JSON" stands for "JavaScript Object Notation"), because of its
>>> simple
>>> and flexible syntax, it eventually found use outside the JavaScript
>>> ecosystem as
>>> a general data interchange format, such as framework configuration files
>>> and web
>>> service requests/response formats.
>>>
>>> While the JDK cannot, and should not, provide libraries for every
>>> conceivable
>>> file format or protocol, the JDK philosophy is one of "batteries included",
>>> which is to say we should be able to write basic programs that use common
>>> protocols such as HTTP, without having to appeal to third party libraries.
>>> The Java ecosystem already has plenty of JSON libraries, so inclusion in
>>> the JDK is largely meant to be a convenience, rather than needing to be the
>>> "one
>>> true" JSON library to meet the needs of all users. Users with specific needs
>>> are always free to select one of the existing third-party libraries.
>>>
>>> ## Goals and requirements
>>>
>>> Our primary goal is that the library be simple to use for parsing,
>>> traversing,
>>> and generating conformant JSON documents. Advanced features, such as data
>>> binding or path-based traversal should be possible to implement as layered
>>> features, but for simplicity are not included in the core API. We adopt a
>>> goal
>>> that the performance should be "good enough", but where performance
>>> considerations conflict with simplicity and usability, we will choose in
>>> favor
>>> of the latter.
>>>
>>> ## API design approach
>>>
>>> The description of JSON at `https:://json.org` describes a JSON document
>>> using
>>> the familiar "railroad diagram":
>>&g
Re: Towards a JSON API for the JDK
> From: "Brian Goetz" > To: "Remi Forax" > Cc: "Paul Sandoz" , "core-libs-dev" > > Sent: Friday, May 16, 2025 7:46:09 PM > Subject: Re: Towards a JSON API for the JDK > If you read the implementation, you'll see that significant laziness is indeed > possible for JsonObject and JsonArray, even while doing eager validation. (Of > course, one can shift the balance to achieve various other tradeoffs.) Reading the implementation is on my TODO list :) Rémi > On 5/16/2025 10:35 AM, [ mailto:fo...@univ-mlv.fr | fo...@univ-mlv.fr ] wrote: >> - Original Message - >>> From: "Brian Goetz" [ mailto:brian.go...@oracle.com | >>> ] >>> To: "Remi Forax" [ mailto:fo...@univ-mlv.fr | ] , "Paul >>> Sandoz" [ mailto:paul.san...@oracle.com | ] Cc: >>> "core-libs-dev" [ mailto:core-libs-dev@openjdk.org | >>> ] Sent: Friday, May 16, 2025 2:53:18 PM >>> Subject: Re: Towards a JSON API for the JDK >>> On 5/15/2025 5:27 PM, Remi Forax wrote: >>>> It's not clear to me why JsonArray (for example) has to be an interface >>>> instead >>>> of a record ? >>> Oh, you know the answer to this. A record limits us to a single >>> implementation with a rigid representation. Behind an interface, we can >>> hide lazy parsing and inflation, wrapping other representations to >>> reduce copies, etc. >> First, let me refine the question. >> There are only 4 kinds of JSON values that benefit from having different >> representations, object, array, string and number. >> For object and array, both takes an interface as parameter (Map or List) so >> JsonArray and JSonObject do not need to be themselves interfaces. >> So the only values where it may be worth to be modeled using an interface are >> JsonString and JsonNumber, because as you said, you can do "lazy" parsing. >> But delaying the parsing of the content has the side effect that even if >> Json.parse() did not throw an exception, it does not mean that the JSON text >> is >> valid, an exception may be thrown later. >> Now, for me, this library is more about being simple and safe than fast. >> If you agree with that, delaying the parsing is not a good idea, thus JSON >> values should be modeled using records and not interfaces. >> regards, >> Rémi
Re: Towards a JSON API for the JDK
> From: "Ethan McCue"
> To: "Remi Forax"
> Cc: "Paul Sandoz" , "core-libs-dev"
>
> Sent: Friday, May 16, 2025 1:44:52 AM
> Subject: Re: Towards a JSON API for the JDK
> I present for your consideration the library I made when spiraling about this
> problem space a few years ago
> [ https://github.com/bowbahdoe/json | https://github.com/bowbahdoe/json ]
> [
> https://javadoc.io/doc/dev.mccue/json/latest/dev.mccue.json/dev/mccue/json/package-summary.html
> |
> https://javadoc.io/doc/dev.mccue/json/latest/dev.mccue.json/dev/mccue/json/package-summary.html
> ]
> Notably missing during the design process here were patterns, hence the
> JsonDecoder design. I haven't been able to evaluate how patterns affect that
> on
> account of them not being out.
> I will more thoroughly peruse the draft of java.util.json at a later date, but
> my initial observations/comments:
> * I am not sure having JsonValue be distinct from Json has value.
> * toUntyped feels a little strange to me - the only type information
> presumably
> lost is the sealed-ness of the hierarchy. The interplay between that and
> toNumber is also a little unnerving.
> * One notion that I found helpful was that a class could be "json encodable,"
> meaning there is a method to call to obtain a canonical json representation.
> record Person(String name) implements JsonEncodable {
> @Override
> public Json toJson() {
> return Json.objectBuilder()
> .put("namen", name)
> .build();
> }
> }
> Which helper methods like Json#of(List) could make
> use
> of. Json itself ( JsonValue in your prototype) could then have a vacuous
> implementation.
> * Terminology wise - I went with reading/writing for the actual
> parsing/generation of json and encoding/decoding for the mapping of those
> representations to/from specific classes. The merits are not top of mind, just
> noting the difference. read/write vs parse/toString+toDisplayString
> * One thing I did was make the helper methods in Json null tolerant and the
> ones
> in the specific subtypes like JsonString not. This was because from what I saw
> of usages of javax.json/jakarta.json that nullability was a footgun and
> correcting for it required changes to code structure (breaking up builder
> chains with if (x != null) checks)
> * The functionality you want from JsonNumber could be achieved by making it
> just
> extend Number ( [
> https://github.com/bowbahdoe/json/blob/main/src/main/java/dev/mccue/json/JsonNumber.java
> |
> https://github.com/bowbahdoe/json/blob/main/src/main/java/dev/mccue/json/JsonNumber.java
> ] ) instead of a bespoke toNumber . You need the extra methods to go to big
> decimal and co, but it's just an extension to the behavior of Number at that
> point.
> * JsonObject and JsonArray could implement Map and List
> respectively. This lowers the need for toUntyped() - since presumably one of
> the use cases for that is turning the json tree into something that more
> generic map/list traversal code can handle. It also complicates any lazy
> loading somewhat.
> * Assuming patterns can be placed on interfaces, you might want to consider
> something similar to JsonDecoder , but with a pattern instead of a method that
> throws an exception.
> // Where here fromJson would box up the logic for testing and extracting from
> each element in the array.
> List people = array(json, Person::fromJson);
> * I don't think there is sufficient cause for anything to be non-sealed at
> this
> point.
> * JsonBoolean and JsonNull do not have reasonable alternative implementations
> -
> as far as I can imagine, maybe i'm wrong - so maybe those can just be final
> classes?
> * If you seal up the whole hierarchy then its pretty trivial to make it
> serializable ( [
> https://github.com/bowbahdoe/json/blob/main/src/main/java/dev/mccue/json/serialization/JsonSerializationProxy.java
> |
> https://github.com/bowbahdoe/json/blob/main/src/main/java/dev/mccue/json/serialization/JsonSerializationProxy.java
> ] )
Hello,
I think your library is more high-level than java.util.json.
Moreover, Paul proposes a design closer to DOP (Data Oriented Programming) than
OOP.
JsonNumber should not extend Number, JsonObject and JsonArray should not
implements Map and List, the idea is to use delegation instead.
(BTW extending Number will become an issue in the future if the VM tries to
flatten filed typed with a value capable abstract class like Number)
JSON is fundamentally untyped and recursive, but also describe data structures,
for me, the proposed API exposes that structure (the subtypes of JsonValue) and
make it easy to move from a JSON text to Java objects or vice-versa.
The java.
Re: Towards a JSON API for the JDK
- Original Message - > From: "Brian Goetz" > To: "Remi Forax" , "Paul Sandoz" > Cc: "core-libs-dev" > Sent: Friday, May 16, 2025 2:53:18 PM > Subject: Re: Towards a JSON API for the JDK > On 5/15/2025 5:27 PM, Remi Forax wrote: >> It's not clear to me why JsonArray (for example) has to be an interface >> instead >> of a record ? > > Oh, you know the answer to this. A record limits us to a single > implementation with a rigid representation. Behind an interface, we can > hide lazy parsing and inflation, wrapping other representations to > reduce copies, etc. First, let me refine the question. There are only 4 kinds of JSON values that benefit from having different representations, object, array, string and number. For object and array, both takes an interface as parameter (Map or List) so JsonArray and JSonObject do not need to be themselves interfaces. So the only values where it may be worth to be modeled using an interface are JsonString and JsonNumber, because as you said, you can do "lazy" parsing. But delaying the parsing of the content has the side effect that even if Json.parse() did not throw an exception, it does not mean that the JSON text is valid, an exception may be thrown later. Now, for me, this library is more about being simple and safe than fast. If you agree with that, delaying the parsing is not a good idea, thus JSON values should be modeled using records and not interfaces. regards, Rémi
Re: Towards a JSON API for the JDK
- Original Message -
> From: "cay horstmann"
> To: "Remi Forax"
> Cc: "core-libs-dev"
> Sent: Monday, May 19, 2025 4:12:55 PM
> Subject: Re: Towards a JSON API for the JDK
> Il 19/05/25 10:13, Remi Forax ha scritto:
>>> If only there was some deconstruction magic that approximates the JavaScript
>>> code
>>>
>>> const doc = { name: "John", age: 30 }
>>> const { name, age } = doc
>>
>> We already have that, it's called a record :)
>>
>> Basically, you are advocating for a mapping JsonObject <--> record.
>>
>> [...]
>>
>>>
>>> Cheers,
>>>
>>> Cay
>>>
>>
>> Here is a simple JsonObject mapper using java.util.json types.
>>https://github.com/forax/json-object-mapper
>
> That's interesting, but I don't think it works as a solution to read generic
> JSON. I have to deal with much JSON that is at best semi-structured. And
> anyway, databinding is is excluded from the scope of the core Java JSON API.
If the mapping is driven by the record definition then mapping and matching are
mostly equivalent,
mapping throws exceptions when something goes wrong while matching returns
something saying no-match.
Here is an example,
record Person(String name, int age) {}
var recordMapper = RecordMapper.of(MethodHandles.lookup());
JsonObject json = ...
if (recordMapper.match(json, Person.class) instanceof Person(String name,
int age)) {
// can use name and age here
}
see
https://github.com/forax/json-object-mapper/blob/master/src/test/java/json/RecordMapperTest.java#L40
[...]
>
> At first I thought the pattern matching version would be worse, but I admit
> that
> the structural safety is appealing.
I agree.
>
> Cheers,
>
> Cay
Rémi
Re: Towards a JSON API for the JDK
> From: "Paul Sandoz" > To: "Remi Forax" > Cc: "Brian Goetz" , "core-libs-dev" > > Sent: Monday, May 19, 2025 10:02:50 PM > Subject: Re: Towards a JSON API for the JDK >> On May 19, 2025, at 8:16 AM, Remi Forax wrote: >> Okay, >> i've taken a look to the design and this is not pretty. > That seems an exaggerated statement to me. It's a trade-off, a compromise, > allowing others to implement their own parsers, perhaps from non-textual > representations. So of course we cannot enforce certain constraints and we > need > to specify how implementations must behave. >> The main issue is that the javadoc claims that >> "Both JsonValue instances and their underlying values are immutable." >> but at the same time any subtypes of JsonValue is non-sealed so anyone can >> implement let say JsonString and adds it's own mutable implementation. You can not claim that JsonValue instances are immutable while obviously they are not. You can not claim that JsonValue subtypes are ADTs while obviously JsonValue is not algebraic. You allow anybody to write their own class inside the JsonValue hierarchy, so there is no safety, strings can be not escaped correctly, number can represent invalid values, etc. The minute I write a method that takes a JsonValue as parameter i'm writing unsafe code, You are hoping that nobody will ever extend JsonValue subtypes, that's wishful thinking, this is not a safe design ... this is not pretty. Rémi
Re: Towards a JSON API for the JDK
> From: "Paul Sandoz" > To: "Remi Forax" > Cc: "Brian Goetz" , "core-libs-dev" > > Sent: Monday, May 19, 2025 11:18:26 PM > Subject: Re: Towards a JSON API for the JDK > Those extending the non-sealed subtypes of JsonValue must conform to the > requirements that are specified. The current documentation could be more > clearly written as to what those requirements are. Of course we cannot enforce > those requirements any more than we can enforce the requirements specified for > implementations of List. List is such a good example. For a List, like for a JsonValue, you have no idea if the implementation is mutable or not, so you can use a defensive copy using List.copyOf() (or Collections.unmodifiableList(new ArrayList<>(list)) in the older version of Java). How to do a defensive copy of a JsonValue ? The other issue I see is lazyness in Json.parse(). The API is restricted to String/char[], so restricted to JSON document that are not big. If the document is not big, lazyness usually make things slower and it also delays the moment you know if the document is a valid JSON document or not. With the actual design, you have no way to know if a JSON document is valid. If you combine both things, the fact that you have a big list of requirements for each subtypes and eager parsing, you are not far from having only one implementation possible, hence the idea of implementing real ADTs using records. > Paul. Rémi >> On May 19, 2025, at 1:56 PM, fo...@univ-mlv.fr wrote: >>> From: "Paul Sandoz" >>> To: "Remi Forax" >>> Cc: "Brian Goetz" , "core-libs-dev" >>> >>> Sent: Monday, May 19, 2025 10:02:50 PM >>> Subject: Re: Towards a JSON API for the JDK >>>> On May 19, 2025, at 8:16 AM, Remi Forax wrote: >>>> Okay, >>>> i've taken a look to the design and this is not pretty. >>> That seems an exaggerated statement to me. It's a trade-off, a compromise, >>> allowing others to implement their own parsers, perhaps from non-textual >>> representations. So of course we cannot enforce certain constraints and we >>> need >>> to specify how implementations must behave. >>>> The main issue is that the javadoc claims that >>>> "Both JsonValue instances and their underlying values are immutable." >>>> but at the same time any subtypes of JsonValue is non-sealed so anyone can >>>> implement let say JsonString and adds it's own mutable implementation. >> You can not claim that JsonValue instances are immutable while obviously they >> are not. >> You can not claim that JsonValue subtypes are ADTs while obviously JsonValue >> is >> not algebraic. >> You allow anybody to write their own class inside the JsonValue hierarchy, so >> there is no safety, strings can be not escaped correctly, number can >> represent >> invalid values, etc. >> The minute I write a method that takes a JsonValue as parameter i'm writing >> unsafe code, >> You are hoping that nobody will ever extend JsonValue subtypes, that's >> wishful >> thinking, this is not a safe design ... this is not pretty. >> Rémi
Re: RFR: 8242888: Convert dynamic proxy to hidden classes
- Original Message - > From: "Chen Liang" > To: "core-libs-dev" , "hotspot-dev" > , kulla-...@openjdk.org > Sent: Thursday, May 23, 2024 1:28:01 PM > Subject: Re: RFR: 8242888: Convert dynamic proxy to hidden classes > On Thu, 23 May 2024 03:28:30 GMT, Chen Liang wrote: > >> Please review this change that convert dynamic proxies implementations to >> hidden >> classes, intended to target JDK 24. >> >> Summary: >> 1. Adds new implementation while preserving the old implementation behind >> `-Djdk.reflect.useLegacyProxyImpl=true` in case there are compatibility >> issues. >> 2. ClassLoader.defineClass0 takes a ClassLoader instance but discards it in >> native code; I updated native code to reuse that ClassLoader for Proxy >> support. >> 3. ProxyGenerator changes mainly involve using Class data to pass Method list >> (accessed in a single condy) and removal of obsolete setup code generation. >> >> Testing: tier1 and tier2 have no related failures. >> >> Comment: Since #8278, Proxy has been converted to ClassFile API, and >> infrastructure has changed; now, the migration to hidden classes is much >> cleaner and has less impact, such as preserving ProtectionDomain and dynamic >> module without "anchor classes", and avoiding java.lang.invoke package. > > A CSR targeting 24 describing the compatibility concerns and behavioral > differences is here, somehow not linked by skara: > https://bugs.openjdk.org/browse/JDK-8332770 > The incompatibilities were much greater in the previous iterations of this > issue, such as in dynamic modules, serialization, and in proxy class > protection > domain. Now these aspects are addressed by this patch, the only real one left > is the change in stack trace. Feel free to raise other incompatibilities you > have discovered. I wonder if instead of using hidden classes, we should not use usual named classes and add a new Lookup.defineClass() that takes a classData as parameter. This will solve the both the problem of the stacktrace and the problem of the roundtrip proxyClass != Class.forName(proxyClass.getName()). Rémi > > - > > PR Comment: https://git.openjdk.org/jdk/pull/19356#issuecomment-2126869679
Re: RFR: 8333265: De-duplicate method references in java.util.stream.FindOps
On Thu, 30 May 2024 12:50:36 GMT, Claes Redestad wrote: > Extracting duplicate method references to static field reduce proxy class > spinning and loading. In this case 2 less classes loaded when using > `findAny()` on each type of stream. For constant method reference, the solution is to use a constant dynamic instead of an invokedynamic. - PR Comment: https://git.openjdk.org/jdk/pull/19477#issuecomment-2139645936
Gatherer and primitive specialization
Hello, the Gatherer API do not provide primitive specialization while intermediate methods like map(), flatMap() or mapMulti() do. Unlike collectors where it is okay to not be specialized because most collectors mutate a collection that is not specialized too, gatherers can process/transform values so having boxing in the middle of those kind of computations is a performance pothole. I think there is a trick that can be used, we do not need to specialize all the functions of the Gatherer API but only the integrator. All other functions are called a few time when a stream is processed so they may not need specialization, only the integrator is called a lot. regards, Rémi
Re: RFR: 8335480: Only deoptimize threads if needed when closing shared arena [v2]
On Fri, 12 Jul 2024 20:59:26 GMT, Jorn Vernee wrote: >> This PR limits the number of cases in which we deoptimize frames when >> closing a shared Arena. The initial intent of this was to improve the >> performance of shared arena closure in cases where a lot of threads are >> accessing and closing shared arenas at the same time (see attached >> benchmark), but unfortunately even disabling deoptimization altogether does >> not have a great effect on that benchmark. >> >> Nevertheless, I think the extra logging/testing/benchmark code, and comments >> I've written, together with reducing the number of cases where we deoptimize >> (which makes it clearer exactly why we need to deoptimize in the first >> place), will be useful going forward. So, I've a create this PR out of them. >> >> In this PR: >> - Deoptimizing is now only done in cases where it's needed, instead of >> always. Which is in cases where we are not inside an `@Scoped` method, but >> are inside compiled code. >> - I've separated the stack walking code (`for_scope_method`) from the code >> that checks for a reference to the arena being closed >> (`is_accessing_session`), and added logging code to the former. That also >> required changing vframe code to accept an `ouputStream*` rather than always >> printing to `tty`. >> - Added a new test (`TestConcurrentClose`), that tries to close many shared >> arenas at the same time, in order to stress that use case. >> - Added a new benchmark (`ConcurrentClose`), that stresses the cases where >> many threads are accessing and closing shared arenas. >> >> I've done several benchmark runs with different amounts of threads. The >> confined case stays much more consistent, while the shared cases balloons up >> in time spent quickly when there are more than 4 threads: >> >> >> Benchmark Threads Mode Cnt Score Error Units >> ConcurrentClose.sharedAccess 32 avgt 10 9017.397 ± 202.870 us/op >> ConcurrentClose.sharedAccess 24 avgt 10 5178.214 ± 164.922 us/op >> ConcurrentClose.sharedAccess 16 avgt 10 2224.420 ± 165.754 us/op >> ConcurrentClose.sharedAccess8 avgt 10 593.828 ± 8.321 us/op >> ConcurrentClose.sharedAccess7 avgt 10 470.700 ± 22.511 us/op >> ConcurrentClose.sharedAccess6 avgt 10 386.697 ± 59.170 us/op >> ConcurrentClose.sharedAccess5 avgt 10 291.157 ± 7.023 us/op >> ConcurrentClose.sharedAccess4 avgt 10 209.178 ± 5.802 us/op >> ConcurrentClose.sharedAccess1 avgt 1052.042 ± 0.630 us/op >> ConcurrentClose.confine... > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > track has_scoped_access for compiled methods Nice work ! Thinking a bit about how to improve the benchmark and given the semantics of Arena.close(), there is a trick that you can use. There are two kinds of memory segments, the one that only visible from Java and the one that are visible not only from Java. By example, a memory segment created from a mmap or a memory segment with an address sent to a C code are visible from outside Java, for those, you have no choice but wait in Arena.close() until all threads have answered to the handshakes. For all the other memory segments, because they are only visible from Java, their memory can be reclaimed asynchronously, i.e. the last thread of the handshakes can free the corresponding memory segments, so the thread that call Arena.close() is free to run even if the memory is not yet reclaimed. >From my armchair, that seems a awful lot of engeneering so it may not worth >it, but now you know :) - PR Comment: https://git.openjdk.org/jdk/pull/20158#issuecomment-2226858328
Re: RFR: 8335480: Only deoptimize threads if needed when closing shared arena [v2]
On Fri, 12 Jul 2024 20:59:26 GMT, Jorn Vernee wrote: >> This PR limits the number of cases in which we deoptimize frames when >> closing a shared Arena. The initial intent of this was to improve the >> performance of shared arena closure in cases where a lot of threads are >> accessing and closing shared arenas at the same time (see attached >> benchmark), but unfortunately even disabling deoptimization altogether does >> not have a great effect on that benchmark. >> >> Nevertheless, I think the extra logging/testing/benchmark code, and comments >> I've written, together with reducing the number of cases where we deoptimize >> (which makes it clearer exactly why we need to deoptimize in the first >> place), will be useful going forward. So, I've a create this PR out of them. >> >> In this PR: >> - Deoptimizing is now only done in cases where it's needed, instead of >> always. Which is in cases where we are not inside an `@Scoped` method, but >> are inside a compiled frame that has a scoped access somewhere inside of it. >> - I've separated the stack walking code (`for_scope_method`) from the code >> that checks for a reference to the arena being closed >> (`is_accessing_session`), and added logging code to the former. That also >> required changing vframe code to accept an `ouputStream*` rather than always >> printing to `tty`. >> - Added a new test (`TestConcurrentClose`), that tries to close many shared >> arenas at the same time, in order to stress that use case. >> - Added a new benchmark (`ConcurrentClose`), that stresses the cases where >> many threads are accessing and closing shared arenas. >> >> I've done several benchmark runs with different amounts of threads. The >> confined case stays much more consistent, while the shared cases balloons up >> in time spent quickly when there are more than 4 threads: >> >> >> Benchmark Threads Mode Cnt Score Error Units >> ConcurrentClose.sharedAccess 32 avgt 10 9017.397 ± 202.870 us/op >> ConcurrentClose.sharedAccess 24 avgt 10 5178.214 ± 164.922 us/op >> ConcurrentClose.sharedAccess 16 avgt 10 2224.420 ± 165.754 us/op >> ConcurrentClose.sharedAccess8 avgt 10 593.828 ± 8.321 us/op >> ConcurrentClose.sharedAccess7 avgt 10 470.700 ± 22.511 us/op >> ConcurrentClose.sharedAccess6 avgt 10 386.697 ± 59.170 us/op >> ConcurrentClose.sharedAccess5 avgt 10 291.157 ± 7.023 us/op >> ConcurrentClose.sharedAccess4 avgt 10 209.178 ± 5.802 us/op >> ConcurrentClose.sharedAccess1 avgt 10 ... > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > track has_scoped_access for compiled methods Knowing that all the segments are freed during close() is something you may want. But having the execution time of close() be linear with the number of threads is also problematic. Maybe, it means that we need another kind of Arena that works like shared() but allow the freed to be done asynchronously (ofSharedAsyncFree ?). Note that the semantics of ofSharedAsyncFree() is different from ofAuto(), ofAuto() relies on the GC to free a segment so the delay before a segment is freed is not time bounded if the application has enough memory, the memory of the segment may never be reclaimed. With ofSharedAsyncFree(), the segments are freed by the last thread, so while this mechanism is not deterministic, it is time bounded. - PR Comment: https://git.openjdk.org/jdk/pull/20158#issuecomment-2226992713
Re: RFR: 8335480: Only deoptimize threads if needed when closing shared arena [v2]
On Mon, 15 Jul 2024 10:50:34 GMT, Jorn Vernee wrote: > This is what I was thinking of as well. close() on a shared arena can be > called by any thread, so it would be possible to have an executor service > with 1-n threads that is dedicated to closing memory. This delays both the closing of the Arena and the freeing of the segments, so bugs may be not discovered if the arena is accessed in between the time the thread pool is notified and the time the close() is effectively called. And you loose the structured part of the API, you can not use a try-with-resources anymore. I think that part can be fixed using a wrapper on top of Arena.ofShared(). - PR Comment: https://git.openjdk.org/jdk/pull/20158#issuecomment-2228280373
Re: RFR: 8335480: Only deoptimize threads if needed when closing shared arena [v3]
On Mon, 15 Jul 2024 11:33:30 GMT, Jorn Vernee wrote: >> This PR limits the number of cases in which we deoptimize frames when >> closing a shared Arena. The initial intent of this was to improve the >> performance of shared arena closure in cases where a lot of threads are >> accessing and closing shared arenas at the same time (see attached >> benchmark), but unfortunately even disabling deoptimization altogether does >> not have a great effect on that benchmark. >> >> Nevertheless, I think the extra logging/testing/benchmark code, and comments >> I've written, together with reducing the number of cases where we deoptimize >> (which makes it clearer exactly why we need to deoptimize in the first >> place), will be useful going forward. So, I've a create this PR out of them. >> >> In this PR: >> - Deoptimizing is now only done in cases where it's needed, instead of >> always. Which is in cases where we are not inside an `@Scoped` method, but >> are inside a compiled frame that has a scoped access somewhere inside of it. >> - I've separated the stack walking code (`for_scope_method`) from the code >> that checks for a reference to the arena being closed >> (`is_accessing_session`), and added logging code to the former. That also >> required changing vframe code to accept an `ouputStream*` rather than always >> printing to `tty`. >> - Added a new test (`TestConcurrentClose`), that tries to close many shared >> arenas at the same time, in order to stress that use case. >> - Added a new benchmark (`ConcurrentClose`), that stresses the cases where >> many threads are accessing and closing shared arenas. >> >> I've done several benchmark runs with different amounts of threads. The >> confined case stays much more consistent, while the shared cases balloons up >> in time spent quickly when there are more than 4 threads: >> >> >> Benchmark Threads Mode Cnt Score Error Units >> ConcurrentClose.sharedAccess 32 avgt 10 9017.397 ± 202.870 us/op >> ConcurrentClose.sharedAccess 24 avgt 10 5178.214 ± 164.922 us/op >> ConcurrentClose.sharedAccess 16 avgt 10 2224.420 ± 165.754 us/op >> ConcurrentClose.sharedAccess8 avgt 10 593.828 ± 8.321 us/op >> ConcurrentClose.sharedAccess7 avgt 10 470.700 ± 22.511 us/op >> ConcurrentClose.sharedAccess6 avgt 10 386.697 ± 59.170 us/op >> ConcurrentClose.sharedAccess5 avgt 10 291.157 ± 7.023 us/op >> ConcurrentClose.sharedAccess4 avgt 10 209.178 ± 5.802 us/op >> ConcurrentClose.sharedAccess1 avgt 10 ... > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > improve benchmark Even if the int vs long issue is fixed for this case, i think we should recommand to call `withInvokeExactBehavior()` after creating any VarHandle so all the auto-conversions are treated as runtime errors. This is what i do with my students (when using compareAndSet) and it makes this kind of perf issue easy to find and easy to fix. - PR Comment: https://git.openjdk.org/jdk/pull/20158#issuecomment-2228979913
Re: New candidate JEP: 431: Sequenced Collections
- Original Message - > From: "mark reinhold" > To: "Stuart Marks" > Cc: "core-libs-dev" , "jdk-dev" > > Sent: Wednesday, October 12, 2022 1:17:20 AM > Subject: New candidate JEP: 431: Sequenced Collections > https://openjdk.org/jeps/431 > > Summary: Introduce new interfaces to represent collections with a > defined encounter order. Each such collection has a well-defined first > element, second element, and so forth, up to the last element. It also > provides uniform APIs for accessing its first and last elements, and > for processing its elements in reverse order. People will again think that i'm the grumpy guy but i prefer to voice my concerns. - nobody cares, i'm back from Devoxx and nobody cares about Sequenced Collections, i've tried to ask several friends what they think about it and the answer was "meh". The bar to introduce new interfaces in the collection API is really really high given how the Java ecosystem works. Once a library starts to use those interfaces as method parameter, it pressures the other library authors to write methods that provides object typed as those interfaces. Not enough people care and the cost for the community (not only Oracle) is high, it looks like a recipe for failure. - LinkedHashMap can be tweaked in two ways, by passing an access order as 3rd parameter of the constructor or by overriding removeEldesEntry(), in both cases the resulting LinkedHashMap is at odds with the contract of SequencedMap but the compiler will happily allow to see those LinkedHashMap as SequencedMap. - LinkedHashMap/LinkedHashSet are dinosaurs, there are more efficient implementations of the concepts of ordered set / ordered map both in term of memory footprint and runtime execution, so adding new interfaces without exploring new implementations using Valhalla value class and in relation with the Collection Literal JEP seems premature to me. > > - Mark Rémi
Re: RFR - Implementation of JEP-430 String Templates (Preview) CSR
Each time i re-read this JEP, i'm more and more sorry about its state.
- we are breaking how Strings worked, now everything in between "..." is not a
String anymore.
As an example, given a String s, only one of the following lines compiles
System.out.println("s = \{s}".toUpperCase());
System.out.println("s = {\s}".toUpperCase());
- conceptually we have something like
instance -> template -> arguments
but instead of grouping it like this
Class.apply(template).invoke(instance, arguments)
the JEP groups them like this
instance.apply(new TemplatedString(template, arguments))
which is far less efficient because both the class and the template are
constants while the instance and the arguments are not.
Rémi
> From: "Jim Laskey"
> To: "core-libs-dev"
> Sent: Tuesday, October 25, 2022 6:41:05 PM
> Subject: RFR - Implementation of JEP-430 String Templates (Preview) CSR
> Request for a broader review of the String Template APIs for JEP 430.
> Summary:
> Enhance the Java programming language with string templates, which are similar
> to string literals but contain embedded expressions. A string template is
> interpreted at run time by replacing each expression with the result of
> evaluating that expression, possibly after further validation and
> transformation. This is a preview language feature and API.
> CSR: [ https://bugs.openjdk.org/browse/JDK-8286021 |
> https://bugs.openjdk.org/browse/JDK-8286021 ]
> JEP: [ https://openjdk.org/jeps/430 | https://openjdk.org/jeps/430 ]
> Thank you.
> — Jim
Re: RFR - Implementation of JEP-430 String Templates (Preview) CSR
I would like to retract the comments i've made below,
because the first point is now moot and for the second one, after some more
thinking, it may be not that bad and at least the API is very simple to use.
Rémi
> From: "Remi Forax"
> To: "Jim Laskey"
> Cc: "core-libs-dev"
> Sent: Wednesday, October 26, 2022 11:25:58 AM
> Subject: Re: RFR - Implementation of JEP-430 String Templates (Preview) CSR
> Each time i re-read this JEP, i'm more and more sorry about its state.
> - we are breaking how Strings worked, now everything in between "..." is not a
> String anymore.
> As an example, given a String s, only one of the following lines compiles
> System.out.println("s = \{s}".toUpperCase());
> System.out.println("s = {\s}".toUpperCase());
> - conceptually we have something like
> instance -> template -> arguments
> but instead of grouping it like this
> Class.apply(template).invoke(instance, arguments)
> the JEP groups them like this
> instance.apply(new TemplatedString(template, arguments))
> which is far less efficient because both the class and the template are
> constants while the instance and the arguments are not.
> Rémi
>> From: "Jim Laskey"
>> To: "core-libs-dev"
>> Sent: Tuesday, October 25, 2022 6:41:05 PM
>> Subject: RFR - Implementation of JEP-430 String Templates (Preview) CSR
>> Request for a broader review of the String Template APIs for JEP 430.
>> Summary:
>> Enhance the Java programming language with string templates, which are
>> similar
>> to string literals but contain embedded expressions. A string template is
>> interpreted at run time by replacing each expression with the result of
>> evaluating that expression, possibly after further validation and
>> transformation. This is a preview language feature and API.
>> CSR: [ https://bugs.openjdk.org/browse/JDK-8286021 |
>> https://bugs.openjdk.org/browse/JDK-8286021 ]
>> JEP: [ https://openjdk.org/jeps/430 | https://openjdk.org/jeps/430 ]
>> Thank you.
>> — Jim
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 17:57:30 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update TemplateRuntime::combine
src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 32:
> 30:
> 31: import java.io.IOException;
> 32: import java.util.*;
Please do not use import *.
src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 36:
> 34: import java.lang.invoke.MethodHandles.Lookup;
> 35: import java.lang.template.StringTemplate;
> 36: import java.util.*;
Another import * here
src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 118:
> 116: * @since 20
> 117: */
> 118: public static final int MAX_INDY_CONCAT_ARG_SLOTS = 200;
I do not think it's a good idea to make that constant available for everybody
given that it's an artefact of the implementation.
src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 999:
> 997: * Promote integral types to int.
> 998: */
> 999: private static Class promoteIntType(Class t) {
promoteToIntType ?
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 17:57:30 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update TemplateRuntime::combine
src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 1042:
> 1040: * The number of fragments must be one more that the number of
> ptypes.
> 1041: * The total number of slots used by the ptypes must be less than
> or equal
> 1042: * to MAX_INDY_CONCAT_ARG_SLOTS.
see my comment about making MAX_INDY_CONCAT_ARG_SLOTS public
src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 1060:
> 1058: throws StringConcatException
> 1059: {
> 1060: Objects.requireNonNull(fragments, "fragments is null");
I think you need to do some defensive copy here
ptypes = List.copyOf(pTypes);
to avoid the types and fragments to be changed at the same time they are
checked.
src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 1177:
> 1175: */
> 1176: @PreviewFeature(feature=PreviewFeature.Feature.STRING_TEMPLATES)
> 1177: public static List makeConcatWithTemplateCluster(
I think instead of having two public methods and the user having to choose
between them, it's better to have the implementations private and on public
methods that calls the correct implementation if maxSlots >
MAX_INDY_CONCAT_ARG_SLOTS or not
src/java.base/share/classes/java/lang/template/ProcessorLinkage.java line 51:
> 49: /**
> 50: * Construct a {@link MethodHandle} that constructs a result based on
> the
> 51: * bootstrap method information.
This comment is quite obscure if you have no idea how it works.
And the information that the returned method handle has the type of the
MethodType passed as parameter is missing.
src/java.base/share/classes/java/lang/template/SimpleStringTemplate.java line
38:
> 36: record SimpleStringTemplate(List fragments,
> 37: List values
> 38: ) implements StringTemplate {}
A compact constructor doing defensive copies is missing
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 17:57:30 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update TemplateRuntime::combine
src/java.base/share/classes/java/lang/template/StringProcessor.java line 45:
> 43: @PreviewFeature(feature=PreviewFeature.Feature.STRING_TEMPLATES)
> 44: @FunctionalInterface
> 45: public interface StringProcessor extends TemplateProcessor {}
The name should be `StringTemplateProcessor`.
And i'm not sure it's useful to have a specialized version for String,
TemplateProcessor is not an issue given that most of the time people
will implement it, so writing `implements StringTemplateProcessor` instead of
`implements TemplateProcessor` does not seem to offer enough bangs for
bucks.
src/java.base/share/classes/java/lang/template/StringTemplate.java line 29:
> 27:
> 28: import java.lang.invoke.MethodHandle;
> 29: import java.util.*;
Please fix.
src/java.base/share/classes/java/lang/template/StringTemplate.java line 149:
> 147: * {@return the interpolation of the StringTemplate}
> 148: */
> 149: default String interpolate() {
I wonder if all the default methods should not be better as static methods
given that they are not the important part of the API but more side information
that may be handy
src/java.base/share/classes/java/lang/template/StringTemplate.java line 175:
> 173: * method {@code processor.process(this)}.
> 174: */
> 175: default R process(ValidatingProcessor
> processor) throws E {
signature should be `ValidatingProcessor processor`
src/java.base/share/classes/java/lang/template/StringTemplate.java line 204:
> 202: * embedded expressions fields, otherwise this method returns
> getters for the values list.
> 203: */
> 204: default public List valueGetters() {
I think i prefer the term accessors instead of getters
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 17:57:30 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update TemplateRuntime::combine
src/java.base/share/classes/java/lang/template/StringTemplate.java line 307:
> 305: Objects.requireNonNull(fragment, "fragments elements must be
> non-null");
> 306: }
> 307: fragments = Collections.unmodifiableList(new
> ArrayList<>(fragments));
I think using `List.copyOf()` is more efficient that
`Collections.unmodifiableList(new ArrayList<>(...))` because there is no copy
if the list is already created with List.of().
src/java.base/share/classes/java/lang/template/StringTemplate.java line 323:
> 321: * @throws NullPointerException fragments or values is null or if
> any of the fragments is null
> 322: */
> 323: public static String interpolate(List fragments,
> List values) {
This method also exists has a static method, having both is a bad idea because
it makes StringTemplate::interpolate a compile error, the compiler has no way
to know that it's the same implementation.
src/java.base/share/classes/java/lang/template/StringTemplate.java line 354:
> 352: * @implNote The result of interpolation is not interned.
> 353: */
> 354: public static final StringProcessor STR = st -> st.interpolate();
Should be `StringTemplate::interpolate`.
src/java.base/share/classes/java/lang/template/TemplateProcessor.java line 38:
> 36: * that do not throw checked exceptions. For example:
> 37: * {@snippet :
> 38: * TemplateProcessor processor = st -> {
This is a good example of why having both way to describe a template processor
of string, TemplateProcessor 43: */
> 44: @PreviewFeature(feature=PreviewFeature.Feature.STRING_TEMPLATES)
> 45: public final class TemplateRuntime {
Why this class is public ? and it should be called `TemplateProcessors` linke
all other classes in Java that store a bunch of static methods (Collections,
Collectors, etc)
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 65:
> 63: * @throws Throwableif linkage fails
> 64: */
> 65: public static CallSite stringTemplateBSM(
I wonder if this method should be moved to a class named
`TemplateProcesorFactory` inside `java.lang.runtime`? Like the all the
bootstrap methods recently added.
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 79:
> 77: MethodType processorGetterType =
> MethodType.methodType(ValidatingProcessor.class);
> 78: ValidatingProcessor processor =
> 79: (ValidatingProcessor Throwable>)processorGetter.asType(processorGetterType).invokeExact();
`ValidatingProcessor` should be enough ? No ?
Not using a "? extends Throwable" here make the type unchecked.
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 88:
> 86: * Manages the boostrapping of {@link ProcessorLinkage} callsites.
> 87: */
> 88: private static class TemplateBootstrap {
This class should be `final`
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 117:
> 115: * Static final processor.
> 116: */
> 117: private final ValidatingProcessor
> processor;
Use `ValidatingProcessor` here
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 145:
> 143: private TemplateBootstrap(MethodHandles.Lookup lookup, String
> name, MethodType type,
> 144: List fragments,
> 145: ValidatingProcessor Throwable> processor) {
Use ValidatingProcessor here
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 211:
> 209: @SuppressWarnings("unchecked")
> 210: public static List toList(E... elements) {
> 211: return Collections.unmodifiableList(Arrays.asList(elements));
This is List.of(), please use List.of() instead
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 17:57:30 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update TemplateRuntime::combine
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 289:
> 287: try {
> 288: MethodHandles.Lookup lookup = MethodHandles.lookup();
> 289: MethodHandle getter =
> lookup.findStatic(TemplateRuntime.class, "getValue",
This should be a constant (using the class holder idiom or not)
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 302:
> 300:
> 301: /**
> 302: * Private ethod used by {@link
> TemplateRuntime#valueGetters(StringTemplate)}
Private "method"
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 325:
> 323: * @throws NullPointerException fragments or values is null or if
> any of the fragments is null
> 324: */
> 325: static String interpolate(List fragments, List
> values) {
I think it should be better to ensure that the caller always call with a
List.of() or a List.copyOf() so null is not a possible element
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 389:
> 387: }
> 388: }
> 389: return new
> SimpleStringTemplate(java.lang.template.TemplateRuntime.toList(fragments),
> java.lang.template.TemplateRuntime.toList(values));
It seems that IntelliJ was lot when auto-completing or doing a refactoring
given that you are alreay in the class TemplateRuntime.
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 17:57:30 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update TemplateRuntime::combine
src/java.base/share/classes/java/util/FormatProcessor.java line 198:
> 196: * {@link FMT} uses the Locale.US {@link Locale}.
> 197: */
> 198: public static final FormatProcessor FMT = new
> FormatProcessor(Locale.US);
`Locale.US` or `Locale.ROOT` ??
see
https://docs.oracle.com/en/java/javase/19/docs/api/java.base/java/util/Locale.html#ROOT
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 19:34:37 GMT, Jim Laskey wrote: >> src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line >> 118: >> >>> 116: * @since 20 >>> 117: */ >>> 118: public static final int MAX_INDY_CONCAT_ARG_SLOTS = 200; >> >> I do not think it's a good idea to make that constant available for >> everybody given that it's an artefact of the implementation. > > There have been several requests to make it public in the past. You really > can't use the methods in this class unless you know the value. Better to have > the value exposed instead of developers transcribing the value into their > code. But it's an implementation details, BTW i wonder if the limitation is still valid, i know that John has changed the implementation of the BSM in that area. - PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 19:51:21 GMT, Franz Wilhelmstötter wrote:
>> src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 211:
>>
>>> 209: @SuppressWarnings("unchecked")
>>> 210: public static List toList(E... elements) {
>>> 211: return Collections.unmodifiableList(Arrays.asList(elements));
>>
>> This is List.of(), please use List.of() instead
>
> `List.of()` can't be used here, since the elements are nullable, according to
> the documentation. But the the returned list can still be modified, by
> changing the given `elements` array. The input array must be explicitly
> copied:
>
> public static List toList(E... elements) {
> return Collections.unmodifiableList(new
> ArrayList<>(Arrays.asList(elements)));
> }
Yes, it only occurs to me mid review, that said there is already an
implementation in the jdk of a compact immutable that allow null inside the JDK
(this implementation is used when stream.toList() is used).
Using that implementation will avoid a bunch of indirection
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 19:52:14 GMT, Rémi Forax wrote: >> There have been several requests to make it public in the past. You really >> can't use the methods in this class unless you know the value. Better to >> have the value exposed instead of developers transcribing the value into >> their code. > > But it's an implementation details, BTW i wonder if the limitation is still > valid, i know that John has changed the implementation of the BSM in that > area. Anyway, i think you are right, this can be public - PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Fri, 28 Oct 2022 20:01:41 GMT, Jim Laskey wrote:
>> src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line
>> 1042:
>>
>>> 1040: * The number of fragments must be one more that the number of
>>> ptypes.
>>> 1041: * The total number of slots used by the ptypes must be less than
>>> or equal
>>> 1042: * to MAX_INDY_CONCAT_ARG_SLOTS.
>>
>> see my comment about making MAX_INDY_CONCAT_ARG_SLOTS public
>
> Disagree.
As i said above, i consider this thread as resolved
>> src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line
>> 1177:
>>
>>> 1175: */
>>> 1176: @PreviewFeature(feature=PreviewFeature.Feature.STRING_TEMPLATES)
>>> 1177: public static List makeConcatWithTemplateCluster(
>>
>> I think instead of having two public methods and the user having to choose
>> between them, it's better to have the implementations private and on public
>> methods that calls the correct implementation if maxSlots >
>> MAX_INDY_CONCAT_ARG_SLOTS or not
>
> Use cases are very different. The first one produces a `MethodHandle` that
> has multiple inputs, The second one produces a `MethodHandle` that can only
> have one input.
yes, sorry for the noise.
>> src/java.base/share/classes/java/lang/template/SimpleStringTemplate.java
>> line 38:
>>
>>> 36: record SimpleStringTemplate(List fragments,
>>> 37: List values
>>> 38: ) implements StringTemplate {}
>>
>> A compact constructor doing defensive copies is missing
>
> The defensive copies are done by the callers.
In that case, i wonder if not not better to move that record inside another
class, closer to where the callers are
>> src/java.base/share/classes/java/lang/template/StringProcessor.java line 45:
>>
>>> 43: @PreviewFeature(feature=PreviewFeature.Feature.STRING_TEMPLATES)
>>> 44: @FunctionalInterface
>>> 45: public interface StringProcessor extends TemplateProcessor {}
>>
>> The name should be `StringTemplateProcessor`.
>> And i'm not sure it's useful to have a specialized version for String,
>> TemplateProcessor is not an issue given that most of the time people
>> will implement it, so writing `implements StringTemplateProcessor` instead
>> of `implements TemplateProcessor` does not seem to offer enough
>> bangs for bucks.
>>
>> see TemplateProcessor
>
> Wrong use case. Think `StringProcessor upper = st ->
> st.interpolate().toUpperCase();`
Is it that different from`TemplateProcessor upper = st ->
st.interpolate().toUpperCase();` ?
People are really used to use <> with the functional interfaces of
java.util.function, and you avoid the "two ways" to express the same thing.
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Sat, 29 Oct 2022 00:56:18 GMT, ExE Boss wrote:
>> src/java.base/share/classes/java/lang/template/StringTemplate.java line 323:
>>
>>> 321: * @throws NullPointerException fragments or values is null or if
>>> any of the fragments is null
>>> 322: */
>>> 323: public static String interpolate(List fragments,
>>> List values) {
>>
>> This method also exists has a static method, having both is a bad idea
>> because it makes StringTemplate::interpolate a compile error, the compiler
>> has no way to know that it's the same implementation.
>
> Actually, `StringTemplate::interpolate` is fine, as this method takes two
> parameters, whereas the instance method only takes an implicit `this`
> parameter.
>
> The instance method is only assignable to `Function`
> or `Supplier`, and the static method is only assignable to
> `BiFunction, List, String>`.
Ok, get it.
I still see not reason to have this method being public given that this is
equivalent to `Template.of(fragments, values).interpolate()`.
The less methods in the API, the better.
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Mon, 31 Oct 2022 13:02:18 GMT, Jim Laskey wrote:
>> src/java.base/share/classes/java/lang/template/StringTemplate.java line 149:
>>
>>> 147: * {@return the interpolation of the StringTemplate}
>>> 148: */
>>> 149: default String interpolate() {
>>
>> I wonder if all the default methods should not be better as static methods
>> given that they are not the important part of the API but more side
>> information that may be handy
>
> Actually instance interpolate() is the most important method. Each synthetic
> StringTemplate gets a specialized interpolate providing performance
> equivalent to string concat. And, a good percentage of processors will work
> with the result of interpolate to produce result. Ex. `StringProcessor STR =
> st -> st.interpolate();` and`TemplateProcessor JSON = st -> new
> JSONObject(st.interpolate());`
Interesting !
I believe the javadoc should mention that.
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v7]
On Mon, 31 Oct 2022 20:11:34 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Add @SafeVarargs declarations
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 132:
> 130: return result;
> 131: }
> 132: for (Object value : st.values()) {
I think that valueTypes() should return the types of the values not the dynamic
classes of the values.
Here there is no type information so it should be Object for all values.
It has also the nice property that the return type of the accessors returned by
valueAccessors are the same as valueTypes() which is i believe more coherent.
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v7]
On Mon, 31 Oct 2022 20:11:34 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Add @SafeVarargs declarations
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 119:
> 117: Class tsClass = st.getClass();
> 118: if (tsClass.isSynthetic()) {
> 119: try {
I do not know if this code is worth of optimizing but the way to avoid to
recompute the List> each time is to use a java.lang.ClassValue and
store the classes inside an unmodifiable List. (Field[] -> Class[] ->
List>) The last leg can be done just by calling List.of(), there is no
need for an ArrayList here
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v7]
On Tue, 1 Nov 2022 18:22:07 GMT, Jim Laskey wrote:
>> src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 119:
>>
>>> 117: Class tsClass = st.getClass();
>>> 118: if (tsClass.isSynthetic()) {
>>> 119: try {
>>
>> I do not know if this code is worth of optimizing but the way to avoid to
>> recompute the List> each time is to use a java.lang.ClassValue and
>> store the classes inside an unmodifiable List. (Field[] -> Class[] ->
>> List>) The last leg can be done just by calling List.of(), there is
>> no need for an ArrayList here
>
> Will use List.of. I think use case is raw and caching should be left to the
> user.
i agree
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: [External] : Re: New candidate JEP: 431: Sequenced Collections
- Original Message - > From: "Remi Forax" > To: "Stuart Marks" > Cc: "core-libs-dev" > Sent: Wednesday, November 2, 2022 7:27:02 PM > Subject: Re: [External] : Re: New candidate JEP: 431: Sequenced Collections There is also the issue with equals/hashCode. You can test the equality of two lists using List.equals(), you can test the equality of two sets using Set.equals(), but you can not compare two SequencedCollection using equals. A SequencedCollection basically occupy the same lattice as Collection and do not provide enough guarantee to be different than Collection. Rémi
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v7]
On Wed, 2 Nov 2022 18:27:30 GMT, Jim Laskey wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransLiterals.java
>> line 429:
>>
>>> 427: }
>>> 428:
>>> 429: private JCClassDecl newStringTemplateClass() {
>>
>> I find it weird to have the compiler emit an implementation of
>> StringTemplate just to capture what is there in the code. If there are many
>> usages of string templates, this could lead to a proliferation of synthetic
>> classes. Perhaps we should consider using a metafactory here, like we do for
>> lambdas, so that we can return some private JDK StringTemplate
>> implementation type.
>
> The main consideration is performance. I spent quite a bit of time playing
> around with different implementations including metafactories (hence the
> carrier class work.) Since a majority of use cases will be STR and FMT, the
> number of classes will likely be just a few per application. Because of the
> change to force processor always, I will be revisiting this during the
> preview period to work on other solutions (I mentioned
> ProcessorFactories/ProcessorBuilders earlier).
@JimLaskey, someone will implement a LOG at some point in the future and you
will get many template classes per application class.
You mention the carrier but i believe you can implement something similar to a
carrier erasing the type of the values but without using method handles to try
to make it type safe and instead pass the real types as a class data of a
hidden class and later as a type argument once the reified generics will be
released.
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v11]
On Wed, 2 Nov 2022 21:53:00 GMT, Jorn Vernee wrote: >> src/java.base/share/classes/jdk/internal/template/TemplateSupport.java line >> 147: >> >>> 145: >>> 146: return support.processWithProcessor(); >>> 147: } >> >> Thinking about this protocol some more: this seems to depend on the >> processor being a constant, which precludes specialized linking of call >> sites where the processor is a dynamic argument. >> >> The returned method handle/call site could maybe be changed to take the >> processor instance as a leading argument instead. The processor can then be >> used to pass additional arguments to the processing code (like the DB >> example in the JEP). (and, AFAICS, that will also allow using calls to this >> BSM as the sole translation strategy for every type of processor, which >> seems more robust if types are changed later on to (un-)implement >> `ProcessorLinkage`) >> >> The `linkage` method could instead be a `static` method, which is somehow >> tied to the type of the processor. Since it's currently a sealed interface >> you could have a mapping from each implementer to the `linkage` method for >> the types you care about (only `FormatProcessor` atm). If that is to be >> opened up to public extension in the future, something like type classes >> would be needed I think, so that the runtime can reliably map from the >> processor type to the static `linkage` method. >> >> WDYT? > > (FWIW, I don't see this as prohibitive for this PR to go ahead, but maybe > something to consider before finalizing the feature) Yes, the current code only supports processors with no fields, which is Okay given that the interface `ProcessorLinkage` is sealed. If/When the processor linkage API is open for everyone as you said the design will have to be changed. But going the other way, creates a general mechanism is painful because either you have an adhoc way to associate the processor to a BSM-like method or you have to delay until runtime the linkage using a monomorphic/polymorphic cache (I know Jim has tested that before back-pedaling to a simpler more constrained good enough solution). - PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v7]
On Thu, 3 Nov 2022 14:37:55 GMT, Jim Laskey wrote: >> src/java.base/share/classes/java/lang/template/ProcessorLinkage.java line 60: >> >>> 58: * @throws NullPointerException if any of the arguments are null >>> 59: */ >>> 60: MethodHandle linkage(List fragments, MethodType type); >> >> I suggest changing the protocol here to be able to take all bootstrap >> arguments into account, and return a `CallSite` instead. That will allow a >> `ProcessorLinkage` to take the lookup and name into account as well, and >> allows returning e.g. a `MutableCallSite` as well. >> >> Maybe this can still be changed later as well though, since the interface is >> sealed. > > Yes - this will all go away during preview. I disagree with Jorn, - CallSite.dynamicInvoker() can be used to see a `MutableCallSite` as a `MethodHandle` so returning a `MethodHandle` is as powerful as returning a `CallSite`. - Having a `ProcessorLinkage` that takes a Lookup as parameter is a security risk because it means that a processor have full access to the user code that calls the processor at runtime. - PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v13]
On Thu, 3 Nov 2022 17:23:53 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Internalize FormatConcatItem
src/java.base/share/classes/java/lang/template/StringTemplate.java line 276:
> 274: * @implNote Contents of both lists are copied to construct
> immutable lists.
> 275: */
> 276: public static StringTemplate of(List fragments, List
> values) {
Should be `StringTemplate of(List fragments, List values) {`
The call to List.copyOf() will change the List to List.
src/java.base/share/classes/java/lang/template/StringTemplate.java line 299:
> 297: * @throws NullPointerException fragments or values is null or if
> any of the fragments is null
> 298: */
> 299: public static String interpolate(List fragments,
> List values) {
Should be `String interpolate(List fragments, List values) {` as
above
src/java.base/share/classes/java/lang/template/StringTemplate.java line 305:
> 303: int valuesSize = values.size();
> 304: if (fragmentsSize != valuesSize + 1) {
> 305: throw new RuntimeException("fragments must have one more
> element than values");
Should be IllegalArgumentException (see method of() above)
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v13]
On Thu, 3 Nov 2022 17:23:53 GMT, Jim Laskey wrote: >> Enhance the Java programming language with string templates, which are >> similar to string literals but contain embedded expressions. A string >> template is interpreted at run time by replacing each expression with the >> result of evaluating that expression, possibly after further validation and >> transformation. This is a [preview language feature and >> API](http://openjdk.java.net/jeps/12). > > Jim Laskey has updated the pull request incrementally with one additional > commit since the last revision: > > Internalize FormatConcatItem src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 159: > 157: } > 158: } > 159: return new > SimpleStringTemplate(TemplateRuntime.toList(fragments), > TemplateRuntime.toList(values)); Should be `return new SimpleStringTemplate(List.copyOf(fragments), TemplateRuntime.toList(values));` because only a value can be null, a fragment should not be null - PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v13]
On Thu, 3 Nov 2022 17:23:53 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Internalize FormatConcatItem
src/java.base/share/classes/java/lang/template/ValidatingProcessor.java line
100:
> 98: * // check or manipulate the fragments and/or values
> 99: * ...
> 100: * return StringTemplate.interpolate(fragments, values);;
There is two semicolons instead of one at the end of this line
src/java.base/share/classes/java/lang/template/ValidatingProcessor.java line
126:
> 124: * }
> 125: * The {@link StringTemplate#interpolate()} method is available for
> those processors
> 126: * that just need to work with the interpolatation;
type `interpolatation`-> `interpolation`
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v13]
On Thu, 3 Nov 2022 17:23:53 GMT, Jim Laskey wrote:
>> Enhance the Java programming language with string templates, which are
>> similar to string literals but contain embedded expressions. A string
>> template is interpreted at run time by replacing each expression with the
>> result of evaluating that expression, possibly after further validation and
>> transformation. This is a [preview language feature and
>> API](http://openjdk.java.net/jeps/12).
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Internalize FormatConcatItem
src/java.base/share/classes/java/lang/template/TemplateRuntime.java line 158:
> 156: values[j++] = value;
> 157: }
> 158: }
The sides effects `fragments[i++] += fragmentIter.next();` and `i--` are ugly
and can easily be overlook.
Also there is no need to use an iterator here, we know that the fragments have
a List.get() in O(1).
The idea is to concat the last fragment of a template with the first one of the
next template, so i propose
...
String[] fragments = new String[size + 1];
String last = "";
int i = 0, j = 0;
for (StringTemplate st : sts) {
List templateFragments = st.fragments();
fragments[i++] = last.concat(templateFragments.get(0)); // concat
last fragment with first new fragment
int k = 0;
for(; k < templateFragments.size() - 1; k++) {
fragments[i++] = templateFragments.get(k);
}
last = templateFragments.get(k);
for (Object value : st.values()) {
values[j++] = value;
}
}
fragments[i] = last;
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
On Wed, 9 Nov 2022 21:12:46 GMT, Jim Laskey wrote: >> Changing > > Raw list? I think this comment is not on the right part of the code. - PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v13]
On Sat, 5 Nov 2022 22:23:23 GMT, Rémi Forax wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Internalize FormatConcatItem
>
> src/java.base/share/classes/java/lang/template/StringTemplate.java line 276:
>
>> 274: * @implNote Contents of both lists are copied to construct
>> immutable lists.
>> 275: */
>> 276: public static StringTemplate of(List fragments,
>> List values) {
>
> Should be `StringTemplate of(List fragments, List values) {`
>
> The call to List.copyOf() will change the List to List.
> raw List ?
oops, formatting issue.
The idea is that `values` can be typed `List` because the call to
List.copyOf() will take the `List` and return a `List`.
And as a type of a parameter `List` is better than `List` because
`List` is a subtype of `List` but not a subtype of `List`.
-
PR: https://git.openjdk.org/jdk/pull/10889
Re: RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v16]
On Thu, 10 Nov 2022 15:09:14 GMT, Jim Laskey wrote: >> Enhance the Java programming language with string templates, which are >> similar to string literals but contain embedded expressions. A string >> template is interpreted at run time by replacing each expression with the >> result of evaluating that expression, possibly after further validation and >> transformation. This is a [preview language feature and >> API](http://openjdk.java.net/jeps/12). > > Jim Laskey has updated the pull request incrementally with one additional > commit since the last revision: > > Clean up new StringTemplate creation > > Centralized StringTemplate creation in StringTemplateImplFactory. Added > consistent defensive copying of lists. Changed List to List on > StringTemplate.interpolate and StringTemplate.of. src/java.base/share/classes/java/lang/runtime/TemplateRuntime.java line 36: > 34: import java.lang.template.StringTemplate; > 35: import java.lang.template.ValidatingProcessor; > 36: import java.util.*; They come back :) - PR: https://git.openjdk.org/jdk/pull/10889
Re: Unsigned long to double and back
Hi all,
I don't know if it's the correct way to do it or if there is a better way, but
i'm using
var result = (double) (value & 0x7fffL );
if (value < 0) {
result = result + 9.223372036854776E18;
}
The idea is to mask the sign bit, do the conversion to double and if the sign
bit was present (if value < 0) add Long.MAX_VALUE + 1 (9223372036854775808) as
a double.
I've got 9.223372036854776E18 as the result of
BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE).doubleValue()
Rémi
> From: "David Lloyd"
> To: "Johannes Kuhn"
> Cc: "core-libs-dev"
> Sent: Friday, November 11, 2022 3:38:31 PM
> Subject: Re: Unsigned long to double and back
> Well, I typed this out from memory so there's an error, of course. `(tmp & 1)`
> should be `(input & 1)`.
> On Fri, Nov 11, 2022 at 8:31 AM David Lloyd < [ mailto:david.ll...@redhat.com
> |
> david.ll...@redhat.com ] > wrote:
>> I encountered this issue as well; for now I'm using the following
>> transformation:
>> long tmp = input >>> 1;
>> double output = ((double) tmp) * 2.0 + (tmp & 1);
>> I... *think* it's correct but I'm not 100% sure and have a long-standing
>> TODO to
>> try and figure it out...
>> On Sat, Nov 5, 2022 at 7:17 PM Johannes Kuhn < [ mailto:i...@j-kuhn.de |
>> i...@j-kuhn.de ] > wrote:
>>> When I tried to implement an WASM transpiler, I noticed some missing
>>> conversion methods from unsigned types to floating point, for example
>>> from unsigned long to a double.
>>> For the meaning of unsigned long, see Long.toUnsignedString(long i).
>>> Converting between unsigned long and floating point is not a trivial
>>> task, as it probably requires some bit manipulation.
>>> In particular, I would love to see the following methods added*:
>>> - double Double.fromUnsignedLong(long i)
>>> - long Double.toUnsignedLong(double d)
>>> - float Float.fromUnsignedLong(long i)
>>> - long Float.toUnsignedLong(float f)
>>> * Subject to bikeshedding - I don't care about the name, or if it is
>>> added to the Long class.
>>> Currently, I don't think that additional methods for unsigned int are
>>> necessary - as it is possible to cast between long and int, but feel
>>> free to correct me.
>>> In WASM, the specification for those methods can be found here:
>>> [ https://www.w3.org/TR/wasm-core-1/#op-trunc-u |
>>> https://www.w3.org/TR/wasm-core-1/#op-trunc-u ]
>>> [ https://www.w3.org/TR/wasm-core-1/#op-convert-u |
>>> https://www.w3.org/TR/wasm-core-1/#op-convert-u ]
>>> Note that the WASM specification is undefined for some values, notably
>>> NaN, infinities, and values that fall out of the range.
>>> As *I* want to use it to implement WASM instructions, I do not have any
>>> strong opinion on the undefined cases - for example, returning the
>>> nearest unsigned long value or throwing an exception is fine for me.
>>> What do you think?
>>> - Johannes
>> --
>> - DML • he/him
> --
> - DML • he/him
Re: RFR: 8299576: Reimplement java.io.Bits using VarHandle access [v7]
On Mon, 9 Jan 2023 09:22:25 GMT, Per Minborg wrote: >> Currently, `java.io.Bits` is using explicit logic to read/write various >> primitive types to/from byte arrays. Switching to the use of `VarHandle` >> access would provide better performance and less code. >> >> Also, using a standard API for these conversions means future `VarHandle` >> improvements will benefit `Bits` too. >> >> Improvements in `Bits` will propagate to `ObjectInputStream`, >> `ObjectOutputStream` and `RandomAccessFile`. >> >> Initial benchmarks and performance discussions can be found here: >> https://github.com/openjdk/panama-foreign/pull/762 > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Remove faulty test tag, improve other test tag, fix comments yes, see https://docs.oracle.com/en/java/javase/19/docs/api/java.base/java/lang/invoke/MethodHandle.html#sigpoly and https://docs.oracle.com/en/java/javase/19/docs/api/java.base/java/lang/invoke/VarHandle.html - PR: https://git.openjdk.org/jdk/pull/11840
Re: RFR: 8300237: Minor improvements in MethodHandles
On Tue, 17 Jan 2023 09:51:31 GMT, Claes Redestad wrote:
>> - `MethodType.ptypes()` can be used instead of `MethodType.parameterList()`
>> when we don't need a copy
>> - comparison of two lists can be done without `Stream.reduce()`
>
> src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 6754:
>
>> 6752: filter(t -> t.parameterCount() > skipSize).
>> 6753: map(MethodType::ptypes).
>> 6754: reduce((p, q) -> p.length >= q.length ? p :
>> q).orElse(EMPTY);
>
> Could avoid the need to introduce `EMPTY` if you make the stream expression
> return the longest list directly:
>
> reduce((p, q) -> {
> var longest = (p.size() >= q.size()) ? p : q;
> return List.of(Arrays.copyOfRange(longest, skipSize,
> longest.size()); // checking isEmpty() is redundant here since we filter on
> `t.parameterCount() > skipSize`
> }).orElse(List.of());
Using lambdas inside MethodHandles is quite dangerous given that lambdas are
initialized using method handles. It may work now because
longuestParameterList() is not called when initializing a lambda but it may
make any changes in the implementation of lambdas painfull in the future.
-
PR: https://git.openjdk.org/jdk/pull/12025
Re: RFR: 8300237: Minor improvements in MethodHandles [v2]
On Tue, 17 Jan 2023 10:18:42 GMT, Claes Redestad wrote: >> Using lambdas inside MethodHandles is quite dangerous given that lambdas are >> initialized using method handles. It may work now because >> longuestParameterList() is not called when initializing a lambda but it may >> make any changes in the implementation of lambdas painfull in the future. > > Precious little method handle use in lambda bootstrap since JDK 11. Though I > agree with the sentiment - having fixed a number of bootstrap issues in the > past - `MethodHandles` is a small step up the abstraction ladder and the code > in particular already uses a number of method refs and lambdas. ok, two small changes, - formatting: usually the method call in a stream are aligned with the '.' at the beginning ``` stream .filter(...) .map(...) ``` instead of at the end. - the reduce is a max(), `max(Comparator.comparingInt(List::size))` - PR: https://git.openjdk.org/jdk/pull/12025
Re: RFR: 8300237: Minor improvements in MethodHandles [v3]
On Tue, 17 Jan 2023 18:07:37 GMT, Sergey Tsypanov wrote: >> - `MethodType.ptypes()` can be used instead of `MethodType.parameterList()` >> when we don't need a copy >> - comparison of two lists can be done without `Stream.reduce()` > > Sergey Tsypanov has updated the pull request incrementally with one > additional commit since the last revision: > > Polishing src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 59: > 57: import java.nio.ByteOrder; > 58: import java.security.ProtectionDomain; > 59: import java.util.*; oops src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 6747: > 6745: .max(Comparator.comparing(MethodType::parameterCount)) > 6746: .map(MethodType::ptypes) > 6747: .map(longest -> List.of(Arrays.copyOfRange(longest, > skipSize, longest.length))) i think you can fuse these to map() calls - PR: https://git.openjdk.org/jdk/pull/12025
Re: RFR: 8300237: Minor improvements in MethodHandles [v5]
On Wed, 18 Jan 2023 07:32:58 GMT, Sergey Tsypanov wrote: >> - `MethodType.ptypes()` can be used instead of `MethodType.parameterList()` >> when we don't need a copy >> - comparison of two lists can be done without `Stream.reduce()` > > Sergey Tsypanov has updated the pull request incrementally with one > additional commit since the last revision: > > Use comparingInt() Looks good to me, i'm not an official reviewer but claes already review this PR so this is fine. - PR: https://git.openjdk.org/jdk/pull/12025
Re: RFR: 8299444 java.util.Set.copyOf allocates needlessly for empty input collections [v2]
On Tue, 24 Jan 2023 10:23:14 GMT, Viktor Klang wrote:
>> src/java.base/share/classes/java/util/ImmutableCollections.java line 169:
>>
>>> 167: @SuppressWarnings("unchecked")
>>> 168: static List listCopy(Collection coll) {
>>> 169: if (coll instanceof List12 || (coll instanceof ListN && !
>>> ((ListN)coll).allowNulls)) {
>>
>> Maybe replace the cast with an instanceof pattern here?
>
> @stuart-marks Sorry, missed this notification. I initially had the same idea,
> but decided against it because it forces me to suppress "rawtypes" since
> `coll instanceof ListN` is not considered to be a rawtype, but `coll
> instanceof ListN c` is. And currently it won't allow for `coll instanceof
> ListN c`...
`coll instanceof ListN list` should work.
-
PR: https://git.openjdk.org/jdk/pull/11847
Re: RFR: 8299444 java.util.Set.copyOf allocates needlessly for empty input collections [v2]
On Tue, 24 Jan 2023 13:40:37 GMT, Viktor Klang wrote: >> `coll instanceof ListN list` should work. > > @forax @stuart-marks Yeah, that works. It's unfortunate that it's not > possible to match on the actual (generic) type, as then both sides of the || > could use type unification to avoid having to do the cast on the return value > as well. Submitted a commit to switch to the wildcard version. Avoiding the cast to `List` requires two things: - we need a way to say at declaration that a parameter type E is covariant, this is not true for List but this is true for ImmutableList (so true for List2 and ListN). - we need a way to do an union between two types when doing pattern matching . This former is still in discussion as a feature of Valhalla (it's a part of Dan Smith Phd), the later is still in discussion as a feature of Amber, so maybe in the future this will be possible is the star aligned. - PR: https://git.openjdk.org/jdk/pull/11847
Re: Math.clamp method?
- Original Message - > From: "Tagir Valeev" > To: "core-libs-dev" > Sent: Wednesday, January 25, 2023 2:41:41 PM > Subject: Math.clamp method? > Hello! > > Quite often it's necessary to clamp a numerical value to a given > range, using the algorithm like this: > int clampedValue = value > max ? max : value < min ? min : value; > or probably > int clampedValue = Math.max(min, Math.min(max, value)); > Some examples in wild: [1] [2] > > These "algorithms" are verbose and error-prone. E.g., many years ago > Math.max and Math.min were mixed in Jenkins CI implementation (later > fixed in [3]). Static analyzers like SonarLint even implement a rule > to catch such kind of mistake [4], but it cannot statically find a > mistake if max and min values are not constant. > > Having a library method for this algorithm will make the code more > readable, and robust. Standard libraries of various languages tend to > implement a function that encapsulates this algorithm. E.g., Kotlin > coerseIn [5], C# Math.Clamp [6], Rust num::clamp [7]. > > I hereby propose to add a similar method to Java Math class. The > signatures could be: > public static int clamp(int value, int min, int max) > public static long clamp(long value, long min, long max) > public static double clamp(double value, double min, double max) > public static float clamp(float value, float min, float max) > > These methods should additionally check that min is not greater than > max (throwing IllegalArgumentException). Probably IAE should be thrown > as well if either min or max is NaN for floating point methods. It can > be discussed what should happen if the value is NaN (I think returning > NaN would be ok, but throwing an exception is also a possibility). > > I can post CSR and contribute the implementation if Core Library team > members agree that it's a good idea. I remember John Rose talking about something very similar to this proposal, there is maybe already a bug for it. Given that the semantics of NaN is clearly defined for Math.max/min (if one of the values is NaN the result is NaN), I don't believe we need a special case here for NaN. The semantics should be, this is equivalent to execute Math.max(min, Math.min(max, value)) So clamp(double) can be implemented using minsd and maxsd on x64, which is already what the VM does. > > With best regards, > Tagir Valeev. regards, Rémi > > [1] > https://github.com/moneymod/moneymod/blob/4d2c7f5be40dbf392e75/src/main/java/wtf/moneymod/client/impl/ui/click/buttons/settings/SliderButton.java#L71 > [2] > https://github.com/androidthings/experiment-expression-flower/blob/f0fd6090e0df1659bad/code/app/src/main/java/com/example/androidthings/VideoProcessor.java#L559 > [3] https://github.com/jenkinsci/jenkins/commit/e00f99251e0b > [4] https://rules.sonarsource.com/java/RSPEC-3065 > [5] https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.ranges/coerce-in.html > [6] > https://learn.microsoft.com/en-us/dotnet/api/system.math.clamp?view=net-7.0 > [7] https://docs.rs/num/0.2.1/num/fn.clamp.html
Re: [Proposal] Make toLowerCase and toUpperCase based on Locale.ROOT by default
> From: "Glavo" > To: "core-libs-dev" > Sent: Thursday, January 26, 2023 1:35:06 PM > Subject: [Proposal] Make toLowerCase and toUpperCase based on Locale.ROOT by > default > At present, the no-parameter toLowerCase and toUpperCase methods of String are > based on the default locale. > I checked all the uses of this method in OpenJDK, and found that most of the > use > cases are suspicious, and even there are some hidden bugs. > For example, I just found that jdk.incubator.vector.LaneType will trigger > assertion failure on Turkish locale, and opened a PR for this problem: >> [ https://github.com/openjdk/panama-vector/pull/210 | >> https://github.com/openjdk/panama-vector/pull/210 ] > In addition to such obvious problems, some use cases behave suspiciously after > calling Locale.setDefault. > I am investigating these problems and preparing to open a PR to solve these > hidden bugs. > In all the third-party libraries I have used, I have never seen the correct > use > of these two methods. > Although the behavior of modifying API methods is destructive, I think it is > worthwhile to consider whether to modify its behavior for such a suspicious > method. > If users need locale-sensitive case conversion, it may be better to explicitly > use Locale.getDefault(). > Using Locale.ROOT as the default value also helps to keep the behavior of > these > two methods consistent with equalsIgnoreCase, > Character.toLowerCase/toUpperCase > and other methods. > This is my rough idea. I hope to get your suggestions. yes, it's a very very common bug, IntelliJ already warns when toLowerCase()/toUpperCase() is used without a Locale. I think the logical nest step is more to deprecate toLowerCase()/toUpperCase() than to change their implementation. regards, Rémi
Re: RFR: 8301220: Return value of toArray() of Сollection types from java.base should be trusted [v2]
On Fri, 27 Jan 2023 13:53:45 GMT, Glavo wrote: >> I checked the `java.base` module, and all the `Collection#toArray()` method >> of collections be implemented correctly. >> >> Their return values can be trusted, so many unnecessary array duplication >> can be eliminated. > > Glavo has updated the pull request incrementally with one additional commit > since the last revision: > > Collections.isTrustedCollection I think the simplest solution is to have a non public interface declared inside java.util. Like java.util.RandomAccess, but non public. The main advantage to use an interface is that you can document it and it's easy to find all the implementations. - PR: https://git.openjdk.org/jdk/pull/12212
Re: RFR: 8301220: Return value of toArray() of Сollection types from java.base should be trusted [v2]
On Fri, 27 Jan 2023 13:53:45 GMT, Glavo wrote: >> I checked the `java.base` module, and all the `Collection#toArray()` method >> of collections be implemented correctly. >> >> Their return values can be trusted, so many unnecessary array duplication >> can be eliminated. > > Glavo has updated the pull request incrementally with one additional commit > since the last revision: > > Collections.isTrustedCollection This may change the startup time a lot because initializing the annotation parser (and the factory around) and creating the annotations is quite slow. - PR: https://git.openjdk.org/jdk/pull/12212
Re: NPE throwing behavior of immutable collections
> From: "Glavo"
> To: "Stuart Marks"
> Cc: "John Hendrikx" , "core-libs-dev"
>
> Sent: Tuesday, January 31, 2023 10:12:24 PM
> Subject: Re: NPE throwing behavior of immutable collections
> I understand that null is prohibited by default, but can we also provide a set
> of factory methods that accept null?
> They can be named like List.ofNullable/copyOfNullable.
It is actually spelt, Arrays.asList() and collection.stream().toList().
Rémi
> On Tue, Jan 31, 2023 at 10:13 AM Stuart Marks < [
> mailto:stuart.ma...@oracle.com
> | stuart.ma...@oracle.com ] > wrote:
>> In this reply I'll focus on the null handling issues in collections.
>> As you've noted, there are really (at least) two distinct issues here:
>> whether a
>> collection permits nulls, and the behavior of contains(null) queries. There
>> have
>> been continual complaints about both, and the issues are somewhat distinct.
>> The collection implementations in the JDK are all over the place with regard
>> to
>> nulls. I believe this is because the original collections and the concurrent
>> collections up through JDK 1.6 or so were mostly done by Josh Bloch and Doug
>> Lea,
>> who disagreed about null handling. They had this exchange in 2006:
>> Doug Lea wrote: [1]
>> > The main reason that nulls aren't allowed in ConcurrentMaps
>> > (ConcurrentHashMaps, ConcurrentSkipListMaps) is that
>> > ambiguities that may be just barely tolerable in non-concurrent
>> > maps can't be accommodated. The main one is that if
>> > map.get(key) returns null, you can't detect whether the
>> > key explicitly maps to null vs the key isn't mapped.
>> > In a non-concurrent map, you can check this via map.contains(key),
>> > but in a concurrent one, the map might have changed between calls.
>> > Further digressing: I personally think that allowing
>> > nulls in Maps (also Sets) is an open invitation for programs
>> > to contain errors that remain undetected until
>> > they break at just the wrong time. (Whether to allow nulls even
>> > in non-concurrent Maps/Sets is one of the few design issues surrounding
>> > Collections that Josh Bloch and I have long disagreed about.)
>> Josh Bloch wrote: [2]
>> > I have moved towards your position over the years. It was probably a
>> > mistake to allow null keys in Maps and null elements in Sets. I'm
>> > still not sure about Map values and List elements.
>> > In other words, Doug hates null more than I do, but over the years
>> > I've come to see it as quite troublesome.
>> (I haven't talked to Josh or Doug about this particular issue recently, so I
>> suppose
>> it's possible their opinions have changed in the intervening time.)
>> I had to decide what to do about nulls when I added the unmodifiable
>> collections
>> in
>> JDK 9. Given that Doug "hates" nulls, and Josh finds them at least quite
>> troublesome, I started from a position of disallowing null members in all the
>> new
>> collections. Most of the unmodifiable collections are still this way (but see
>> below).
>> What about contains(null)? Surely it should be ok to query for a null, and
>> return
>> false, even if the collection itself can't contain null. However, a bunch of
>> collections and collection views in the JDK throw NPE on contains(null), so
>> the
>> unmodifiable collections don't set a precedent here.
>> If you're dealing with all non-null values, and a null creeps in somehow,
>> then
>> calling contains(null) might be an error, and it would be good for the
>> library
>> to
>> inform you of that instead of just returning false and letting the program
>> continue
>> until it fails later (fail-fast). A similar issue arises with the non-generic
>> contains() method. If you have a Collection, then shouldn't
>> contains("abc")
>> be an error? It's not, and it simply returns false, because the signature is
>> contains(Object). But there have been complaints that this should have been
>> an
>> error
>> because a Collection of Integer cannot possibly contain a String. Similar
>> reasoning
>> applies to contains(null) on a collection that can't contain null. (Yes, the
>> error
>> occurs at runtime instead of compile time, but better late than never.)
>> A meta-issue is: should a new thing correct a perceived design flaw in the
>> older
>> thing, or should it be consistent with the old thing? Either way, you lose.
>> Another factor in my original decision was that it's much easier to start
>> with a
>> restriction and relax it later than it is to go the other way. We have some
>> flexibility to allow nulls and contains(null) if we want; but it's
>> essentially a
>> non-starter to disallow nulls once they've been allowed somewhere.
>> That's why my starting position for the design prohibited nulls everywhere.
>> Over time we've relaxed some restrictions around null handling in the
>> unmodifiable
>> collections. Streams permit nulls, and the List returned from Stream.toList()
>> also
>> permits nulls, and it also allows contains(null). S
Re: RFR: 8302154: Hidden classes created by LambdaMetaFactory can't be unloaded [v2]
- Original Message -
> From: "Volker Simonis"
> To: "core-libs-dev" , "hotspot-dev"
>
> Sent: Friday, February 10, 2023 8:03:47 PM
> Subject: Re: RFR: 8302154: Hidden classes created by LambdaMetaFactory can't
> be unloaded [v2]
> On Fri, 10 Feb 2023 17:29:37 GMT, Ioi Lam wrote:
>
>>> Volker Simonis has updated the pull request incrementally with two
>>> additional
>>> commits since the last revision:
>>>
>>> - Remove assertions which insist on Lambda proxy classes being strongly
>>> linked
>>> to their class loader
>>> - Removed unused import of STRONG und updated copyright year
>>
>> Even in JDK 11, a lambda proxy classes that's referenced by the cpCache
>> (i.e.,
>> from a resolved invokedynamic instruction associated with a lamda expression)
>> is always kept alive. See test below.
>>
>> So if I understand correctly, this patch will not affect lamda expressions in
>> Java source code. It affects only direct calls to
>> `LambdaMetafactory.metafactory()`. Is this correct?
>>
>>
>> public class LambdaGC {
>> public static void main(String[] args) throws Throwable {
>> System.out.println("Entering LambdaGC");
>> doit(() -> {
>> Thread.dumpStack();
>> });
>> for (int i = 0; i < 10; i++) {
>> System.gc();
>> }
>> System.out.println("Finish LambdaGC");
>> }
>> static void doit(Runnable r) {
>> r.run();
>> }
>> }
>>
>> $ java11 -cp . -XX:+UnlockDiagnosticVMOptions -XX:+ShowHiddenFrames
>> -Xlog:class+load -Xlog:class+unload LambdaGC | grep LambdaGC
>> [0.022s][info][class,load] LambdaGC source: file:/jdk3/tmp/
>> Entering LambdaGC
>> [0.024s][info][class,load] LambdaGC$$Lambda$1/0x000840060840 source:
>> LambdaGC
>> java.lang.Exception: Stack trace
>> at java.base/java.lang.Thread.dumpStack(Thread.java:1387)
>> at LambdaGC.lambda$main$0(LambdaGC.java:5)
>> at LambdaGC$$Lambda$1/0x000840060840.run(:100)
>> at LambdaGC.doit(LambdaGC.java:13)
>> at LambdaGC.main(LambdaGC.java:4)
>> Finish LambdaGC
>
> @iklam, I think your understanding is correct. While the bootstrap methods for
> Java Lambdas do call `LambdaMetafactory.metafactory()`, they store the
> resulting call site (for Lambdas a `BoundMethodHandle`) in the appendix slot
> of
> the constant pool cache entry of the invokedynamic bytecode. The
> `BoundMethodHandle` contains a reference to an instance of the generated
> lambda
> form (i.e. in your example `LambdaGC$$Lambda$1`). This is enough in order to
> keep `LambdaGC$$Lambda$1` alive and prevent its unloading.
>
> Until now, `LambdaGC$$Lambda$1` was also strongly linked to its defining class
> loader, but I don't think that's necessary to keep it alive (because it is
> referenced from the call site which is referenced from the constant pool
> cache).
>
> Running your example with `-Xlog:indy+methodhandles=debug` confirms this:
>
> set_method_handle bc=186 appendix=0x00062b821838 method=0x0008000c7698
> (local signature)
> {method}
> - this oop: 0x0008000c7698
> - method holder: 'java/lang/invoke/Invokers$Holder'
> ...
> appendix: java.lang.invoke.BoundMethodHandle$Species_L
> {0x00062b821838} - klass: 'java/lang/invoke/BoundMethodHandle$Species_L'
> - fields (total size 5 words):
> ...
> - final 'argL0' 'Ljava/lang/Object;' @32 a
> 'LambdaGC$$Lambda$1+0x000801000400'{0x00062b81e080} (0xc5703c10)
Hi Volker,
the main issue if the link is not STRONG is that the VM creates one classloader
data per lambda as Mandy said, if you have a library full of lambdas, this will
consume a lot of C memory which is always hard to to diagnose.
I remember people from RedHat reverting a library code that was 'lambdaified'
because of that.
So it's a kind of pick your poison situation and i believe the current
tradeoff, you need a classloader if you want to unload something (classes or
lambdas) vs you may consume too much off heap memory is the good one.
regards,
Rémi
>
> -
>
> PR: https://git.openjdk.org/jdk/pull/12493
Rémi
Re: RFR: 8302154: Hidden classes created by LambdaMetaFactory can't be unloaded [v2]
On Thu, 9 Feb 2023 18:11:18 GMT, Volker Simonis wrote: >> Prior to >> [JDK-8239384](https://bugs.openjdk.org/browse/JDK-8239384)/[JDK-8238358](https://bugs.openjdk.org/browse/JDK-8238358) >> LambdaMetaFactory has created VM-anonymous classes which could easily be >> unloaded once they were not referenced any more. Starting with JDK 15 and >> the new "hidden class" based implementation, this is not the case any more, >> because the hidden classes will be strongly tied to their defining class >> loader. If this is the default application class loader, these hidden >> classes can never be unloaded which can easily lead to Metaspace exhaustion >> (see the [test case in the JBS >> issue](https://bugs.openjdk.org/secure/attachment/102601/LambdaClassLeak.java)). >> This is a regression compared to previous JDK versions which some of our >> applications have been affected from when migrating to JDK 17. >> >> The reason why the newly created hidden classes are strongly linked to their >> defining class loader is not clear to me. JDK-8239384 mentions it as an >> "implementation detail": >> >>> *4. the lambda proxy class has the strong relationship with the class >>> loader (that will share the VM metaspace for its defining loader - >>> implementation details)* >> >> From my current understanding the strong link between a hidden class created >> by `LambdaMetaFactory` and its defining class loader is not strictly >> required. In order to prevent potential OOMs and fix the regression compared >> the JDK 14 and earlier I propose to create these hidden classes without the >> `STRONG` option. >> >> I'll be happy to add the test case as JTreg test to this PR if you think >> that would be useful. > > Volker Simonis has updated the pull request incrementally with two additional > commits since the last revision: > > - Remove assertions which insist on Lambda proxy classes being strongly > linked to their class loader > - Removed unused import of STRONG und updated copyright year Hi Volker, in the future, we may change the implementation of lambdas again, by example Valhalla parametric generics, Isolated methods (JDK-8158765), JDK-8087219 or JDK-8001537, may allow to re-use the same class file/method for all lambdas implementing the same interface. I fear that forcing unloading may block future refactoring. - PR: https://git.openjdk.org/jdk/pull/12493
Re: RFR: 8302154: Hidden classes created by LambdaMetaFactory can't be unloaded [v2]
On Thu, 9 Feb 2023 18:11:18 GMT, Volker Simonis wrote: >> Prior to >> [JDK-8239384](https://bugs.openjdk.org/browse/JDK-8239384)/[JDK-8238358](https://bugs.openjdk.org/browse/JDK-8238358) >> LambdaMetaFactory has created VM-anonymous classes which could easily be >> unloaded once they were not referenced any more. Starting with JDK 15 and >> the new "hidden class" based implementation, this is not the case any more, >> because the hidden classes will be strongly tied to their defining class >> loader. If this is the default application class loader, these hidden >> classes can never be unloaded which can easily lead to Metaspace exhaustion >> (see the [test case in the JBS >> issue](https://bugs.openjdk.org/secure/attachment/102601/LambdaClassLeak.java)). >> This is a regression compared to previous JDK versions which some of our >> applications have been affected from when migrating to JDK 17. >> >> The reason why the newly created hidden classes are strongly linked to their >> defining class loader is not clear to me. JDK-8239384 mentions it as an >> "implementation detail": >> >>> *4. the lambda proxy class has the strong relationship with the class >>> loader (that will share the VM metaspace for its defining loader - >>> implementation details)* >> >> From my current understanding the strong link between a hidden class created >> by `LambdaMetaFactory` and its defining class loader is not strictly >> required. In order to prevent potential OOMs and fix the regression compared >> the JDK 14 and earlier I propose to create these hidden classes without the >> `STRONG` option. >> >> I'll be happy to add the test case as JTreg test to this PR if you think >> that would be useful. > > Volker Simonis has updated the pull request incrementally with two additional > commits since the last revision: > > - Remove assertions which insist on Lambda proxy classes being strongly > linked to their class loader > - Removed unused import of STRONG und updated copyright year Volker, i think you are mixing lambda metafactory and hidden classes. Hidden Classes have been envision has a way to generate adapters at runtime for languages that run on the JVM. They replace VM anonymous classes that were both unsafe and always not bound to a classloader. Lambda metafactory is the entry point to create lambda proxy for Java (the language). The current implementation is using hidden classes to represent lambda proxy. In the past, it was using VM anonymous classes that why lambdas were not tie to a classloader, the fact that the implementation was using VM anonymous class was leaking. To unload a Java class, its clasloader has to be unreachable. I see no reason this behavior should not be the same for Java lambdas classes. I believe it's what David is saying too. Now, for https://github.com/aws/aws-sdk-java-v2/issues/3701, you can fix it by using a ClassValue to cache the class and/or using a hidden class. For me, this has nothing to do with Java lambdas, i.e. nothing to do with the lambda metafactory. - PR: https://git.openjdk.org/jdk/pull/12493
Re: RFR: 8302154: Hidden classes created by LambdaMetaFactory can't be unloaded [v2]
On Wed, 22 Feb 2023 17:05:00 GMT, Kasper Nielsen wrote: >> Volker Simonis has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Remove assertions which insist on Lambda proxy classes being strongly >> linked to their class loader >> - Removed unused import of STRONG und updated copyright year > > In all the places I've seen LambdaMetaFactory used, it is because of > performance over reflection/non-static method handles. See, for example, > https://www.optaplanner.org/blog/2018/01/09/JavaReflectionButMuchFaster.html. > I believe Optaplanner is still using it. > > There is also quite a number of posts on StackOverflow on people trying to > use LambdaMetafactory: > https://stackoverflow.com/search?q=LambdaMetafactory @kaspernielsen, i believe that now that hidden class + class data are part of the public API, you do not need to use lambda metafactory directly. But it requires Java 17 not 8 or 11. - PR: https://git.openjdk.org/jdk/pull/12493
Re: JEP-198 - Lets start talking about JSON
> From: "Brian Goetz"
> To: "Ethan McCue" , "core-libs-dev"
>
> Sent: Tuesday, February 28, 2023 8:48:00 PM
> Subject: Re: JEP-198 - Lets start talking about JSON
> As you can probably imagine, I've been thinking about these topics for quite a
> while, ever since we started working on records and pattern matching. It
> sounds
> like a lot of your thoughts have followed a similar arc to ours.
> I'll share with you some of our thoughts, but I can't be engaging in a
> detailed
> back-and-forth right now -- we have too many other things going on, and this
> isn't yet on the front burner. I think there's a right time for this work, and
> we're not quite there yet, but we'll get there soon enough and we'll pick up
> the ball again then.
> To the existential question: yes, there should be a simpler, built-in way to
> parse JSON. And, as you observe, the railroad diagram in the JSON spec is a
> graphical description of an algebraic data type. One of the great simplifying
> effects of having algebraic data types (records + sealed classes) in the
> language is that many data modeling problems collapse down to the point where
> considerably less creativity is required of an API. Here's the JSON API one
> can
> write after literally only 30 seconds of thought:
>> sealed interface JsonValue {
>> record JsonString (String s)implements JsonValue { }
>> record JsonNumber (double d)implements JsonValue { }
>> record JsonNull ()implements JsonValue { }
>> record JsonBoolean ( boolean b)implements JsonValue { }
>> record JsonArray (List< JsonValue > values)implements JsonValue { }
>> record JsonObject (Map pairs)implements JsonValue { }
>> }
> It matches the JSON spec almost literally, and you can use pattern matching to
> parse a document. (OK, there's some tiny bit of creativity here in that
> True/False have been collapsed to a single JsonBoolean type, but you get my
> point.)
> But, we're not quite ready to put this API into the JDK, because the language
> isn't *quite* there yet. Records give you nice pattern matching, but they come
> at a cost; they're very specific and have rigid ideas about initialization,
> which ripples into a number of constraints on an implementation (i.e., much
> harder to parse lazily.) So we're waiting until we have deconstruction
> patterns
> (next up on the patterns parade) so that the records above can be interfaces
> and still support pattern matching (and more flexibility in implementation,
> including using value classes when they arrive.) It's not a long hop, though.
> I agree with your assessment of streaming models; for documents too large to
> fit
> into memory, we'll let someone else provide a specialized solution. Streaming
> and fully-materialized-tree are not the only two options; there are plenty of
> points in the middle.
> As to API idioms, these can be layered. The lazy-tree model outlined above can
> be a foundation for data binding, dynamic mapping to records, jsonpath, etc.
> But once you've made the streaming-vs-materialized choice in favor of
> materialized, it's hard to imagine not having something like the above at the
> base of the tower.
> The question you raise about error handling is one that infuses pattern
> matching
> in general. Pattern matching allows us to collapse what would be a thousand
> questions -- "does key X exist? is it mapped to a number? is the number in the
> range of byte?" -- each with their own failure-handling path, into a single
> question. That's great for reliable and readable code, but it does make errors
> more opaque, because it is more like the red "check engine" light on your
> dashboard. (Something like JSONPath could generate better error messages since
> you've given it a declarative description of an assumed structural invariant.)
> But, imperative code that has to treat each structural assumption as a
> possible
> control-flow point is a disaster; we've seen too much code like this already.
> The ecosystem is big enough that there will be lots of people with strong
> opinions that "X is the only sensible way to do it" (we've already seen
> X=databinding on this thread), but the reality is that there are multiple
> overlapping audiences here, and we have to be clear which audiences we are
> prioritizing. We can have that debate when the time is right.
> So, we'll get there, but we're waiting for one or two more bits of language
> evolution to give us the substrate for the API that feels right.
> Hope this helps,
> -Brian
You can "simulate" deconstructors by using when + instanceof,
Let say we an interface with a deconstructor that can deconstruct the instance
of that interface as a tuple of points
interface Point {
record $(int x, int y) {}
$ deconstructor();
}
If there is an implementation, the deconstructor is just an implementation of
an instance method "deconstructor"
class PointImpl implements Point {
private int x;
private int y;
public PointImpl(int x, int y) {
this.x = x;
this.y =
Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v46]
On Thu, 23 Mar 2023 15:49:37 GMT, Jim Laskey wrote: >> On the flip side, for those that don't use `var`; >> >> >> Processor simple = Processor.of(st-> new >> JSONObject(st.interpolate())); >> Processor string = >> Processor.of(StringTemplate::interpolate); >> >> >> vs. >> >> >> SimpleProcessor simple = st-> new JSONObject(st.interpolate()); >> StringProcessor string = StringTemplate::interpolate; > > It's coming back to me why we didn't do this in the first place (these > projects tend to go on for months and years). `SimpleProcessor` exists > because of that ugly second parameter, `E`, in `Processor`, when a > majority of the time `E` will be the unchecked `RuntimeException`. > `StringProcessor` exists because 90+% of template processors will produce > strings. From there, we originally had `Process.of`, `SimpleProcessor.of` and > `StringProcessor.of`. We realized that the `@FunctionalInterface` route was > cleaner and there was no need for `of` -- keep interfaces simple. I would > argue that if you are creating a template processor, it is better to expose > the result type and not bury in a `var`. Not a lot of people will write a processor and among those few, most of them will create a class that implement `Processor` (to have a proper name and a place to put documentation) so providing several reified names (`SimpleProcessor` and `StringProcessor`) is not that useful for implementers. For users, it's not something necessary to understand how processors work or how a specific processor should be used. It looks like a loose loose situation for me, implementers do not need them and users will find them confusing (especially the difference between a processor and a simple processor). - PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1146450918
Re: RFR: JDK-8285932 Implementation of JEP 430 String Templates (Preview) [v46]
On Thu, 23 Mar 2023 12:17:55 GMT, Jim Laskey wrote:
>> src/java.base/share/classes/java/lang/runtime/TemplateRuntime.java line 204:
>>
>>> 202: Object[] values
>>> 203: ) throws Throwable {
>>> 204: List asList = Collections.unmodifiableList(new
>>> ArrayList<>(Arrays.asList(values)));
>>
>> Suggestion:
>>
>> List asList = List.of(values);
>>
>> For defensive copy.
>> Don't think processors are harmed by the null-hostile behavior of these
>> list, for many template implementations already use null-hostile lists.
>
> The values list can't be null-hostile for the same reason that string
> concatenation can't be null-hostile. Please point to examples of null-hostile
> lists (other that fragments) being used in the template code. Apologies if I
> misinterpreted your meaning.
There is a trick to do a defensive copy in case the list can contains a null,
you can use `stream.toList()`,
so
List asList = Arrays.stream(values).toList();
is what you are looking for.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/10889#discussion_r1146455491
Re: The non-deterministic iteration order of Immutable Collections
- Original Message - > From: "Kasper Nielsen" > To: "Chris Hegarty" > Cc: "core-libs-dev" > Sent: Friday, March 24, 2023 6:53:51 PM > Subject: Re: The non-deterministic iteration order of Immutable Collections >> >> I don't (yet) want to be prescriptive in any potential solution. And I >> know that this has been discussed before. I mostly just want to start a >> conversation and see how much traction it gets. >> > Would > java.util.SequencedMap.of(...) > java.util.SequencedMap.copyOf(SequencedMap map) > java.util.SequencedSet.of(...) > java.util.SequencedSet.copyOf(SequencedSet set) > solve your problem? > > I would love to see them included in JEP 431. > Should be fairly simply to implement just have a side > array that maintains the elements in order. I do not think SequenceMap or SequencedSet should be use for copyOf(), Map and Collection should be used instead because the iteration order is enough. This is sadly not something acknowledged by JEP 431, but SequencedSet or SequencedMap are not very useful as interfaces for typing the parameters of public methods. Like NavigableSet/NavigableMap, they can be handy for typing a specific implementation for a field or a local variable but for methods, they are not general enough. > > /Kasper regards, Rémi
Re: RFR: 8266571: Sequenced Collections [v2]
On Sat, 25 Mar 2023 03:54:23 GMT, Stuart Marks wrote:
>> PR for Sequenced Collections implementation.
>
> Stuart Marks has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - More specification tweaks.
> - Add simple overrides to ArrayList.
src/java.base/share/classes/java/util/ArrayList.java line 573:
> 571: } else {
> 572: Object[] es = elementData;
> 573: sz--;
This line and the following two lines can be understood as the field size being
updated which is not the case.
I think it's more readable to use
@SuppressWarnings("unchecked") E oldValue = (E) es[sz - 1];
fastRemove(es, sz - 1);
The JIT will common the subexpression 'sz - 1' at runtime.
src/java.base/share/classes/java/util/Collections.java line 1173:
> 1171: * @serial include
> 1172: */
> 1173: static class UnmodifiableSequencedCollection extends
> UnmodifiableCollection
`private` is missing
-
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148319735
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148319899
Re: RFR: 8266571: Sequenced Collections [v2]
On Sat, 25 Mar 2023 03:54:23 GMT, Stuart Marks wrote:
>> PR for Sequenced Collections implementation.
>
> Stuart Marks has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - More specification tweaks.
> - Add simple overrides to ArrayList.
src/java.base/share/classes/java/util/Collections.java line 1184:
> 1182:
> 1183: @SuppressWarnings("unchecked")
> 1184: private SequencedCollection rc() {
I suggest to use 'delegate' as name instead of 'rc' (no idea what 'rc' means)
src/java.base/share/classes/java/util/Collections.java line 6014:
> 6012: */
> 6013: public static SequencedSet
> newSequencedSetFromMap(SequencedMap map) {
> 6014: if (! map.isEmpty())
This line does an implicit NPE check, either make it explicit using
requireNonNull or at least add a comment
src/java.base/share/classes/java/util/Collections.java line 6023:
> 6021: */
> 6022: private static class SequencedSetFromMap extends SetFromMap
> implements SequencedSet {
> 6023: private final E nsee(Map.Entry e) {
`static` instead of `final`
-
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148320373
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148320720
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148320942
Re: RFR: 8266571: Sequenced Collections [v2]
On Sat, 25 Mar 2023 03:54:23 GMT, Stuart Marks wrote:
>> PR for Sequenced Collections implementation.
>
> Stuart Marks has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - More specification tweaks.
> - Add simple overrides to ArrayList.
src/java.base/share/classes/java/util/LinkedHashSet.java line 297:
> 295: */
> 296: public SequencedSet reversed() {
> 297: class ReverseLinkedHashSetView extends AbstractSet implements
> SequencedSet {
This class should be declared `static` (and private) which means it should not
be declared inside reversed.
src/java.base/share/classes/java/util/LinkedHashSet.java line 313:
> 311: boolean present = LinkedHashSet.this.contains(e);
> 312: LinkedHashSet.this.addFirst(e);
> 313: return ! present;
why there is a space between '!' and 'present' given that you have removed this
space in another change above ?
src/java.base/share/classes/java/util/LinkedList.java line 1293:
> 1291: @SuppressWarnings("serial")
> 1292: static class ReverseOrderLinkedListView extends LinkedList
> implements java.io.Externalizable {
> 1293: final LinkedList list;
Using 3 different fields feel ugly given it seems you only need one.
Why do you not use the casting strategy you are using for the other views ?
src/java.base/share/classes/java/util/List.java line 802:
> 800: */
> 801: default E getFirst() {
> 802: if (this.isEmpty())
weirdly, sometimes you use braces around the ''if and sometimes you don't ?
src/java.base/share/classes/java/util/ReverseOrderListView.java line 60:
> 58: }
> 59:
> 60: ReverseOrderListView(List list, boolean modifiable) {
should be 'private' to force users to use 'of'
src/java.base/share/classes/java/util/ReverseOrderListView.java line 191:
> 189: if (o == this)
> 190: return true;
> 191: if (!(o instanceof List))
should be `if (!(o instanceof List list))`
src/java.base/share/classes/java/util/ReverseOrderListView.java line 209:
> 207: int hashCode = 1;
> 208: for (E e : this)
> 209: hashCode = 31*hashCode + (e==null ? 0 : e.hashCode());
`31 * hashCode` instead of `31*hashCode`
src/java.base/share/classes/java/util/ReverseOrderSortedMapView.java line 44:
> 42: public static SortedMap of(SortedMap map) {
> 43: if (map instanceof ReverseOrderSortedMapView) {
> 44: return ((ReverseOrderSortedMapView)map).base;
use `map instanceof ReverseOrderSortedMapView view` instead to remove the
ugly cast below
src/java.base/share/classes/java/util/ReverseOrderSortedMapView.java line 215:
> 213: static Iterator descendingValueIterator(SortedMap
> map) {
> 214: return new Iterator<>() {
> 215: Iterator keyIterator = descendingKeyIterator(map);
`private final` or better declare it above as a local variable and capture it
inside the anonymous class
src/java.base/share/classes/java/util/ReverseOrderSortedMapView.java line 329:
> 327: K prevKey = null;
> 328: boolean dead = false;
> 329: Iterator> it = descendingEntryIterator(base);
see above about declare it as a local variable, all other fields should be
`private`
src/java.base/share/classes/java/util/ReverseOrderSortedSetView.java line 61:
> 59: return true;
> 60:
> 61: if (!(o instanceof Set))
use `if (!(o instanceof Set set))` to avoid the cast below
-
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148321554
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148321635
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148321852
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148322049
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148322292
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148322490
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148322511
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148322728
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148322895
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148323316
PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1148323602
Re: RFR: 8266571: Sequenced Collections [v2]
On Sat, 25 Mar 2023 03:54:23 GMT, Stuart Marks wrote: >> PR for Sequenced Collections implementation. > > Stuart Marks has updated the pull request incrementally with two additional > commits since the last revision: > > - More specification tweaks. > - Add simple overrides to ArrayList. This change is absolutely massive, implementing reversed() basically doubles the number of implementations which means multiple years of debugging / spec fixing. Reversing a List makes sense, reversing a LinkedHashSet/LinkedHashMap is a nice to have. Having the concept of first and last (getFirst()/getLast()/etc) on Collection is something long awaited. I understand that wanting to separate the concept of Collection and SequencedCollection can be conceptually nice, but multiplying the number of interfaces also multiplies the number of implementations. Pragmatically, this patch is too big compared to how useful it is. I get that Oracle is rich, but this JEP is not only a burden for Oracle but for the whole community. - PR Comment: https://git.openjdk.org/jdk/pull/7387#issuecomment-1483761783
Re: The non-deterministic iteration order of Immutable Collections
- Original Message - > From: "Jens Lideström" > To: "core-libs-dev" > Sent: Sunday, March 26, 2023 11:38:07 AM > Subject: Re: The non-deterministic iteration order of Immutable Collections > I think Map#of and friends would be more useful and less error prone if they > where to return collections that have a fixed iteration order, where the order > is defined by the insertion order when the map is created. I agree. They are several use cases for Set.of()/Map.of(), for testing you want them to not have an order but for a defensive copy (Set.copyOf()/Map.copyOf()) you want them to keep the order. Currently, the implementation rotates toward the former instead of the later. A lot of my students struggle with the semantics of Set.of()/Map.of() because this choice makes the unmodifiable set/map not beginners friendly. To the point where a student will prefer to use a List instead of a Set, because List.copyOf() seems to work "correctly" compared to Set.copyOf(). Python 3.7 has changed the implementation of its set and dictionaries to keep the insertion order for this exact reason. Java could do the same. regards, Rémi
Re: RFR: 6983726: remove Proxy from MethodHandleProxies.asInterfaceInstance SAM conversion
On Mon, 27 Mar 2023 23:34:52 GMT, Chen Liang wrote:
> As John Rose has pointed out in this issue, the current j.l.r.Proxy based
> implementation of MethodHandleProxies.asInterface has a few issues:
> 1. Exposes too much information via Proxy supertype (and WrapperInstance
> interface)
> 2. Does not allow future expansion to support SAM[^1] abstract classes
> 3. Slow (in fact, very slow)
>
> This patch addresses all 3 problems:
> 1. It implements proxies with one hidden class for each requested interface
> and replaced WrapperInstance inheritance with an annotation. This can avoid
> unexpected passing of `instanceof`, and avoids the nasty problem of exporting
> a JDK interface to a dynamic module to ensure access.
> 2. This patch obtains already generated classes from a ClassValue by the
> requested interface type; the ClassValue can later be updated to compute
> implementation generation for abstract classes as well.
> 3. This patch's generated hidden classes has acceptable call and creation
> performance compared to the baseline; though the methods to access wrapper
> information see huge performance drops, they are not anticipated to be used
> in a very frequent basis, while the old implementation's wrapper access
> methods are more optimized (2ns/op) than interface implementation methods
> (6ns/op). [Oracle JDK 20 vs
> this](https://jmh.morethan.io/?gists=bf98de7b2128e7e5d14e697fd9921eb9,e5115a2a8fa0a45159e15fab0d95b5d8)
>
> Additionally, an obsolete `ProxyForMethodHandle` test was removed, for it's
> no longer applicable. Tests in `jdk/java/lang/invoke` and
> `jdk/java/lang/reflect` pass.
>
> Alternative implementation:
> [An alternative
> implementation](https://github.com/openjdk/jdk/commit/72dbf9d4e01c455854d9b865cb2a47c38f37a8e0)
> was to generate a proxy class for each methodhandle than sharing across
> methodhandles. That implementation was abandoned for its bad proxy creation
> performance, despite it having excellent call performance. [Alternative
> implementation vs
> this](https://jmh.morethan.io/?gists=08abb39f224574550925beb8be1b2f59,e5115a2a8fa0a45159e15fab0d95b5d8)
>
> In addition, I have a question: in
> [8161245](https://bugs.openjdk.org/browse/JDK-8161245) it seems some fields
> can be optimized as seen in
> [ciField.cpp](https://github.com/openjdk/jdk/blob/6aec6f3a842ead30b26cd31dc57a2ab268f67875/src/hotspot/share/ci/ciField.cpp#L219).
> Does it affect the execution performance of MethodHandle in hidden classes'
> Condy vs. MethodHandle in regular final field in hidden classes?
>
> [^1]: single abstract method
src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line 206:
> 204: try {
> 205: proxy = (Object) info.ctor.invokeExact(mhs); // non-varargs
> 206: } catch (Throwable e) {
At least Error should be directly propagated (especially OutOfMemoryError)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1150405757
Re: RFR: 6983726: remove Proxy from MethodHandleProxies.asInterfaceInstance SAM conversion
On Mon, 27 Mar 2023 23:34:52 GMT, Chen Liang wrote: > As John Rose has pointed out in this issue, the current j.l.r.Proxy based > implementation of MethodHandleProxies.asInterface has a few issues: > 1. Exposes too much information via Proxy supertype (and WrapperInstance > interface) > 2. Does not allow future expansion to support SAM[^1] abstract classes > 3. Slow (in fact, very slow) > > This patch addresses all 3 problems: > 1. It implements proxies with one hidden class for each requested interface > and replaced WrapperInstance inheritance with an annotation. This can avoid > unexpected passing of `instanceof`, and avoids the nasty problem of exporting > a JDK interface to a dynamic module to ensure access. > 2. This patch obtains already generated classes from a ClassValue by the > requested interface type; the ClassValue can later be updated to compute > implementation generation for abstract classes as well. > 3. This patch's generated hidden classes has acceptable call and creation > performance compared to the baseline; though the methods to access wrapper > information see huge performance drops, they are not anticipated to be used > in a very frequent basis, while the old implementation's wrapper access > methods are more optimized (2ns/op) than interface implementation methods > (6ns/op). [Oracle JDK 20 vs > this](https://jmh.morethan.io/?gists=bf98de7b2128e7e5d14e697fd9921eb9,e5115a2a8fa0a45159e15fab0d95b5d8) > > Additionally, an obsolete `ProxyForMethodHandle` test was removed, for it's > no longer applicable. Tests in `jdk/java/lang/invoke` and > `jdk/java/lang/reflect` pass. > > Alternative implementation: > [An alternative > implementation](https://github.com/openjdk/jdk/commit/72dbf9d4e01c455854d9b865cb2a47c38f37a8e0) > was to generate a proxy class for each methodhandle than sharing across > methodhandles. That implementation was abandoned for its bad proxy creation > performance, despite it having excellent call performance. [Alternative > implementation vs > this](https://jmh.morethan.io/?gists=08abb39f224574550925beb8be1b2f59,e5115a2a8fa0a45159e15fab0d95b5d8) > > In addition, I have a question: in > [8161245](https://bugs.openjdk.org/browse/JDK-8161245) it seems some fields > can be optimized as seen in > [ciField.cpp](https://github.com/openjdk/jdk/blob/6aec6f3a842ead30b26cd31dc57a2ab268f67875/src/hotspot/share/ci/ciField.cpp#L219). > Does it affect the execution performance of MethodHandle in hidden classes' > Condy vs. MethodHandle in regular final field in hidden classes? > > [^1]: single abstract method I believe you can have better performance if you pass the method handle as the class data of the hidden class and you load it with a constant dynamic https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#defineHiddenClassWithClassData(byte[],java.lang.Object,boolean,java.lang.invoke.MethodHandles.Lookup.ClassOption...) and https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/lang/invoke/MethodHandles.html#classData(java.lang.invoke.MethodHandles.Lookup,java.lang.String,java.lang.Class) - PR Comment: https://git.openjdk.org/jdk/pull/13197#issuecomment-1486668297
Re: RFR: 6983726: remove Proxy from MethodHandleProxies.asInterfaceInstance SAM conversion
On Mon, 27 Mar 2023 23:34:52 GMT, Chen Liang wrote:
> As John Rose has pointed out in this issue, the current j.l.r.Proxy based
> implementation of MethodHandleProxies.asInterface has a few issues:
> 1. Exposes too much information via Proxy supertype (and WrapperInstance
> interface)
> 2. Does not allow future expansion to support SAM[^1] abstract classes
> 3. Slow (in fact, very slow)
>
> This patch addresses all 3 problems:
> 1. It implements proxies with one hidden class for each requested interface
> and replaced WrapperInstance inheritance with an annotation. This can avoid
> unexpected passing of `instanceof`, and avoids the nasty problem of exporting
> a JDK interface to a dynamic module to ensure access.
> 2. This patch obtains already generated classes from a ClassValue by the
> requested interface type; the ClassValue can later be updated to compute
> implementation generation for abstract classes as well.
> 3. This patch's generated hidden classes has acceptable call and creation
> performance compared to the baseline; though the methods to access wrapper
> information see huge performance drops, they are not anticipated to be used
> in a very frequent basis, while the old implementation's wrapper access
> methods are more optimized (2ns/op) than interface implementation methods
> (6ns/op). [Oracle JDK 20 vs
> this](https://jmh.morethan.io/?gists=bf98de7b2128e7e5d14e697fd9921eb9,e5115a2a8fa0a45159e15fab0d95b5d8)
>
> Additionally, an obsolete `ProxyForMethodHandle` test was removed, for it's
> no longer applicable. Tests in `jdk/java/lang/invoke` and
> `jdk/java/lang/reflect` pass.
>
> Alternative implementation:
> [An alternative
> implementation](https://github.com/openjdk/jdk/commit/72dbf9d4e01c455854d9b865cb2a47c38f37a8e0)
> was to generate a proxy class for each methodhandle than sharing across
> methodhandles. That implementation was abandoned for its bad proxy creation
> performance, despite it having excellent call performance. [Alternative
> implementation vs
> this](https://jmh.morethan.io/?gists=08abb39f224574550925beb8be1b2f59,e5115a2a8fa0a45159e15fab0d95b5d8)
>
> In addition, I have a question: in
> [8161245](https://bugs.openjdk.org/browse/JDK-8161245) it seems some fields
> can be optimized as seen in
> [ciField.cpp](https://github.com/openjdk/jdk/blob/6aec6f3a842ead30b26cd31dc57a2ab268f67875/src/hotspot/share/ci/ciField.cpp#L219).
> Does it affect the execution performance of MethodHandle in hidden classes'
> Condy vs. MethodHandle in regular final field in hidden classes?
>
> [^1]: single abstract method
Let suppose you have an interface like:
interface StringConsumer extends Consumer {}
The implementation needs to override both accept(Object) and accept(String).
-
PR Comment: https://git.openjdk.org/jdk/pull/13197#issuecomment-1486908491
