Re: RFR: 8247532: Records deserialization is slow

2020-06-16 Thread Peter Levart
Hi Chris, On 6/17/20 12:31 AM, Peter Levart wrote: I did however notice that there is just a single test, DefaultValuesTest, that exercises different stream shapes for the same class in the stream. Would be good to expand coverage in this area, but of course some lower-level test-specific

Re: RFR: 8247532: Records deserialization is slow

2020-06-16 Thread Peter Levart
Hi Remi, The keys used are based on the ordered names and types of "fields" in the ObjectStreamClass that has been deserialized as part of the object stream for the record "type" being deserialized. So for each record type (Class) there can be several distinct ObjectStreamClasses

Re: (15) RFR: JDK-8247444: Trust final fields in records

2020-06-16 Thread John Rose
On Jun 15, 2020, at 2:58 PM, Mandy Chung wrote: > > Webrev: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8247444/webrev.00/ > > CSR: https://bugs.openjdk.java.net/browse/JDK-8247517 >

Re: RFR(S) : 8211977 : move testlibrary tests into one place

2020-06-16 Thread David Holmes
On 17/06/2020 2:45 am, Igor Ignatyev wrote: Hi David, thanks for your review. re: LingeredAppTest, I agree that the test doesn't look very useful, yet I'd remind that the goal of this (and other test in /test/lib-test) is to (sanity) test testlibrary in order to easily spot bugs in

Re: RFR: 8247532: Records deserialization is slow

2020-06-16 Thread Paul Sandoz
> On Jun 16, 2020, at 3:36 PM, Peter Levart wrote: > > Yes, or even better: MethodHandles.empty(MethodType.methodType(pClass, > byte[].class, Object[].class)) which was suggested by Johanes Kuhn as here: > > > http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.02/ >

Re: (15) RFR: JDK-8247444: Trust final fields in records

2020-06-16 Thread Mandy Chung
Hi David, Thanks for the review. On 6/15/20 7:29 PM, David Holmes wrote: Hi Mandy, Christoph, The code changes here all look okay to me. The idea of introducing the notion of "trusted final fields" seems quite reasonable. I made one editorial comment on the CSR request. Thanks. I'm

Re: RFR: 8247532: Records deserialization is slow

2020-06-16 Thread Remi Forax
Hi Peter, is their a reason to not use a ClassValue using the record class as key instead of the more complex keys you are proposing ? Rémi - Mail original - > De: "Chris Hegarty" > À: "Peter Levart" > Cc: "core-libs-dev" > Envoyé: Mardi 16 Juin 2020 17:15:03 > Objet: Re: RFR:

Re: RFR: JDK-8264244: BasicShortcutHintTest shortcut can not be found

2020-06-16 Thread Andy Herrick
looks good with these links /Andy On 6/16/2020 6:13 PM, Alexander Matveev wrote: Hi Alexey, Looks good. I think you got links and bug ID incorrect. It should be JDK-8246244 and you have 8264244. Links also does not work. Working links are: https://bugs.openjdk.java.net/browse/JDK-8246244

Re: RFR: 8247532: Records deserialization is slow

2020-06-16 Thread Peter Levart
Hi Paul, On 6/16/20 5:50 PM, Paul Sandoz wrote: Hi Peter, 199 private Map deserializationCtrs; Change to be either ConcurrentMap or ConcurrentHashMap, thereby making it clearer when using. Sure. Will do that. 2679 private static MethodHandle

Re: RFR: 8247532: Records deserialization is slow

2020-06-16 Thread Peter Levart
Hi Chris, On 6/16/20 5:15 PM, Chris Hegarty wrote: Hi Peter, On 14 Jun 2020, at 17:28, Peter Levart wrote: ... [2] http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.01/ I think that this is very good. It’s clever to build a chain of method handles that can be

RFR: JDK-8264244: BasicShortcutHintTest shortcut can not be found

2020-06-16 Thread Alexey Semenyuk
Please review fix [2] for jpackage bug [1]. The fix is to put value of `Exec` property of .desktop files generated by jpackage in double quotes if the value of the propery contains whitespace characters. - Alexey [1] https://bugs.openjdk.java.net/browse/JDK-8264244 [2] 

Re: RFR: JDK-8264244: BasicShortcutHintTest shortcut can not be found

2020-06-16 Thread Alexander Matveev
Hi Alexey, Looks good. I think you got links and bug ID incorrect. It should be JDK-8246244 and you have 8264244. Links also does not work. Working links are: https://bugs.openjdk.java.net/browse/JDK-8246244 http://cr.openjdk.java.net/~asemenyuk/8246244/webrev.00/ Thanks, Alexander On

RFR: JDK-8213214: Set -Djava.io.tmpdir= when running tests

2020-06-16 Thread Erik Joelsson
(re-sending this as it doesn't look like it was delivered) There are a lot of jtreg tests that use temporary files. These temporary files add up over time and fill up the global temp directories on our test systems. To tackle this, we should try to redirect these temporary files into a

RFR: JDK-8213214: Set -Djava.io.tmpdir= when running tests

2020-06-16 Thread Erik Joelsson
There are a lot of jtreg tests that use temporary files. These temporary files add up over time and fill up the global temp directories on our test systems. To tackle this, we should try to redirect these temporary files into a directory controlled by the test framework. Jtreg does not do

Re: RFR(S) : 8211977 : move testlibrary tests into one place

2020-06-16 Thread Igor Ignatyev
Magnus, Erik, thanks for your reviews, pushed w/ a newline being added at L#654 of make/Main.gmk. Cheers, -- Igor > On Jun 16, 2020, at 7:03 AM, Magnus Ihse Bursie > wrote: > > On 2020-06-16 15:06, Erik Joelsson wrote: >> >> On 2020-06-15 17:39, Igor Ignatyev wrote: >>> @David, Erik,

Re: RFR(S) : 8211977 : move testlibrary tests into one place

2020-06-16 Thread Igor Ignatyev
Hi David, thanks for your review. re: LingeredAppTest, I agree that the test doesn't look very useful, yet I'd remind that the goal of this (and other test in /test/lib-test) is to (sanity) test testlibrary in order to easily spot bugs in testlibrary in a clear manner so one would not have to

Re: (15) RFR: JDK-8247444: Trust final fields in records

2020-06-16 Thread Mandy Chung
On 6/16/20 2:49 AM, Remi Forax wrote: Hi Mndy, Looks good, I don't like too much the fact that there is a new boolean field in j.l.r.Field instead of using the modifiers like in the VM counterpart, but it will require masking and unmasking the modifiers which is a far bigger change, too big

Re: RFR[16]: 8247681: Improve bootstrapping of unary concatenations

2020-06-16 Thread Paul Sandoz
Looks good. It’s tempting to do something like this to de-dup the code with less potential for mistakes: String s = null; // Select the singular non-null value, if any If (s0 != null && s1 == null) s = s0; else if (s0 == null && s1 != null) s = s1; If (s != null) { if (s.isEmpty()) {

Re: RFR: 8247532: Records deserialization is slow

2020-06-16 Thread Paul Sandoz
Hi Peter, 199 private Map deserializationCtrs; Change to be either ConcurrentMap or ConcurrentHashMap, thereby making it clearer when using. 2679 private static MethodHandle defaultValueExtractorFor(Class pType) { Can the implementation use MethodHandles,zero? e.g. return

Re: RFR: 8247532: Records deserialization is slow

2020-06-16 Thread Chris Hegarty
Hi Peter, > On 14 Jun 2020, at 17:28, Peter Levart wrote: > > ... > > [2] > http://cr.openjdk.java.net/~plevart/jdk-dev/RecordsDeserialization/webrev.01/ > I think that this is very good. It’s clever to build a chain of method handles that can be invoked with the stream field values. This

Re: [PATCH] 8246633: Improve the performance of ObjectInputStream.resolveClass(ObjectStreamClass)

2020-06-16 Thread Roger Riggs
Hi Peter, I think you've hardwired an assumption about the contents of the stack into VM.latestUserDefineLoader... On 6/12/20 7:19 PM, Peter Kessler (Open Source) wrote: Roger, I did think about confining the changes to ObjectInputStream. Maybe I did not think hard enough about it.

Re: RFR(S) : 8211977 : move testlibrary tests into one place

2020-06-16 Thread Magnus Ihse Bursie
On 2020-06-16 15:06, Erik Joelsson wrote: On 2020-06-15 17:39, Igor Ignatyev wrote: @David, Erik, Magnus, please find the answers to your comments at the bottom of this email. @all, David's and Erik's comments made me realize that some parts of the original patch were stashed away and

Re: RFR[16]: 8247681: Improve bootstrapping of unary concatenations

2020-06-16 Thread Jim Laskey
> On Jun 16, 2020, at 10:20 AM, Claes Redestad > wrote: > > On 2020-06-16 15:09, Jim Laskey wrote: >> Would it be helpful to have benchmarks for the other types, just in case? > > I'm not sure.. We need to be selective or we end up increasing expected > run time by a large factor. We should

Re: RFR[16]: 8247681: Improve bootstrapping of unary concatenations

2020-06-16 Thread Claes Redestad
On 2020-06-16 15:09, Jim Laskey wrote: Would it be helpful to have benchmarks for the other types, just in case? I'm not sure.. We need to be selective or we end up increasing expected run time by a large factor. We should also do a pass over most of these micros to provide saner defaults

Re: RFR[16]: 8247681: Improve bootstrapping of unary concatenations

2020-06-16 Thread Jim Laskey
Would it be helpful to have benchmarks for the other types, just in case? Don't see any tests to ensure all cases are covered. May be relying on existing tests, but... minimally tag those tests. Cheers, -- Jim > On Jun 16, 2020, at 10:00 AM, Claes Redestad > wrote: > > Hi, > > this

Re: RFR(S) : 8211977 : move testlibrary tests into one place

2020-06-16 Thread Erik Joelsson
On 2020-06-15 17:39, Igor Ignatyev wrote: @David, Erik, Magnus, please find the answers to your comments at the bottom of this email. @all, David's and Erik's comments made me realize that some parts of the original patch were stashed away and didn't make it to webrev.00. I'm truly sorry

RFR[16]: 8247681: Improve bootstrapping of unary concatenations

2020-06-16 Thread Claes Redestad
Hi, this patch specializes bootstrapping of unary concatenation expressions. Such expressions can be reduced to the canonical String.valueOf "stringifier" for any primitive argument, but needs a special stringifier for reference arguments since we need to produce a new String to be compliant

Re: (15) RFR: JDK-8247444: Trust final fields in records

2020-06-16 Thread Remi Forax
Hi Mndy, Looks good, I don't like too much the fact that there is a new boolean field in j.l.r.Field instead of using the modifiers like in the VM counterpart, but it will require masking and unmasking the modifiers which is a far bigger change, too big to worth it in my opinion. So +1 Rémi

Re: RFR(S) : 8211977 : move testlibrary tests into one place

2020-06-16 Thread David Holmes
Hi Igor, On 16/06/2020 10:39 am, Igor Ignatyev wrote: @David, Erik, Magnus, please find the answers to your comments at the bottom of this email. @all, David's and Erik's comments made me realize that some parts of the original patch were stashed away and didn't make it to webrev.00. I'm