Re: RFR: JDK-8197594: String#repeat

2018-03-01 Thread Jim Laskey
Roger, Claes did some performance tests against an earlier implementation where I was doing just that. We determined that the performance difference was lost in the noise. Cheers, — Jim > On Mar 1, 2018, at 10:11 AM, Roger Riggs wrote: > > Hi Jim, > > Perhaps

Re: RFR: 8198888: Reduce string allocation churn in InvokerBytecodeGenerator

2018-03-01 Thread Andrej Golovnin
Hi Claes, src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java 96 /** Internal name lookup cache. */ 97 private HashMap nameMap; 617 nameMap = new HashMap<>(2); Have you tested it with IdentityHashMap? Maybe it would be even better

Re: RFR: 8198888: Reduce string allocation churn in InvokerBytecodeGenerator

2018-03-01 Thread Peter Levart
Hi Claes,  623 name = c.getName().replace('.', '/'); Did you know that there are 319 occurrences of .replace('.', '/') in JDK sources and tests (45 occurrences in java.base without tests)? Isn't that enough to create a special API on the java.lang.Class that would cache this or

Re: RFR: 8198888: Reduce string allocation churn in InvokerBytecodeGenerator

2018-03-01 Thread Claes Redestad
That few, huh? Well, if you ask me the VM internal name and the java class names should have been synced up long ago. :-) I'd not be averse to some real API point in a future RFE. I think this has been dodged in the past to avoid leaking the VM internal representation everywhere. /Claes On

Re: RFR: JDK-8197594: String#repeat

2018-03-01 Thread Jim Laskey
Peter, Very reasonable and worth considering. The main reason the power of 2 copy works well is that the source is (almost) always cached. I thought about this a bit at the beginning and wondered about introducing Arrays.fill(T[] dst, T[] src) where dst is filled repeatedly from src. This can

Re: RFR: JDK-8197594: String#repeat

2018-03-01 Thread Roger Riggs
Hi Jim, Perhaps premature optimization... Have you done any JMH tests on the expected string content and sizes.  The optimization for single 8-bit characters is good but for short strings like inserting spaces for tab stops, "    ".repeat(5), will suffer the overhead of Arraycopy for very

Re: RFR: JDK-8197594: String#repeat

2018-03-01 Thread Claes Redestad
Yes, I think you can squeeze out a 10% improvement or so on some combinations of shortness of String and lowness of repeat count, but at the cost of adding another branch, risking costs due to profile pollution, decreasing chance the method gets inlined etc.. it didn't seem worth the trouble

RFR: JDK-8144300 : Java does not honor http.nonProxyHosts value having wildcard * both at end and start

2018-03-01 Thread Pallavi Sonal
Hi, Please review the following change to fix JDK-8144300. Bug : https://bugs.openjdk.java.net/browse/JDK-8144300 Webrev : http://cr.openjdk.java.net/~rpatil/8144300/webrev.00/ Whenever there is a wildcard(*) at either the beginning or the end of a hostname specified in the list of

Re: RFR: JDK-8197594: String#repeat

2018-03-01 Thread Peter Levart
Hi, On 03/01/2018 03:13 AM, Paul Sandoz wrote: Hi Jim, Looks good. I like the power of 2 copying. Is this really the fastest way? Say you are doing this: String s = ... 64 bytes ... s.repeat(16384); ...the last arraycopy will be copying 512 KiB from one part of memory to the other part.

Re: RFR: 8198888: Reduce string allocation churn in InvokerBytecodeGenerator

2018-03-01 Thread Claes Redestad
Hi, as equals/hashCode on Class mirrors are already using identity semantics, I don't think we actually gain much from using an IdentityHashMap, and I don't want to add new dependencies to this java.lang.invoke (we have some ambition to reduce the number of java.util classes used from

RFR: 8198888: Reduce string allocation churn in InvokerBytecodeGenerator

2018-03-01 Thread Claes Redestad
Hi, two trivial optimizations that get rid of a few percent on ISC startup tests. Webrev: http://cr.openjdk.java.net/~redestad/819/jdk.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-819 Thanks! /Claes

Re: JDK-8197594: String#repeat

2018-03-01 Thread Bernd Eckenfels
Hello, I would not encourage makeshift number formatting by mentioning left padding as a major usecase: Just remove that part: + * + * This method may be used to create space padding for + * formatting text or zero padding for formatting numbers. I think this error without an

Re: JDK-8197594: String#repeat

2018-03-01 Thread Jim Laskey
Thank you Bernd. > On Mar 1, 2018, at 11:04 AM, Bernd Eckenfels wrote: > > Hello, > > I would not encourage makeshift number formatting by mentioning left padding > as a major usecase: > > Just remove that part: > > + * > + * This method may be used to

Re: 8198888: Reduce string allocation churninInvokerBytecodeGenerator

2018-03-01 Thread Claes Redestad
Hi, On 2018-03-01 20:45, Bernd Eckenfels wrote: Cool, the lastClass varient then is pretty tempting (and I wont fix the multi-slot Version below, it misses a „lastUsedSlot“ field) yes, for the startup metrics I'm concerned with then a single element implementation is even ever so slightly

Re: 8198888: Reduce string allocation churninInvokerBytecodeGenerator

2018-03-01 Thread mandy chung
On 3/1/18 12:56 PM, Claes Redestad wrote: yes, for the startup metrics I'm concerned with then a single element implementation is even ever so slightly better: http://cr.openjdk.java.net/~redestad/819/jdk.01/ Looks good to me. Mandy

Re: RFR: JDK-8197594: String#repeat

2018-03-01 Thread Jim Laskey
Ivan, The code is now checked in if you want to test. Cheers, — Jim > On Mar 1, 2018, at 4:20 PM, Ivan Gerasimov wrote: > > Hi Jim! > > I think the copying-loop can be slightly simplified, like this: > > for (; copied < limit - copied; copied <<= 1) { >

Re: RFR: 8198888: Reduce string allocation churn inInvokerBytecodeGenerator

2018-03-01 Thread Bernd Eckenfels
Another question is it safe to keep an unbounded strong reference to classes here? Maybe better Cache getName() results? -- http://bernd.eckenfels.net Von: Paul Sandoz Gesendet: Donnerstag, 1. März 2018 19:05 An: Claes Redestad Cc: core-libs-dev Betreff: Re: RFR: 819: Reduce string

Re: RFR JDK-8187653: Lock in CoderResult.Cache becomes performance bottleneck

2018-03-01 Thread Roger Riggs
Hi Sherman, Looks to be a good fix with predictable behavior and performance. +1, Roger On 2/23/2018 8:26 PM, Xueming Shen wrote: Hi, Please help review the proposed change to remove the potential performance bottleneck in CoderResult caused by the "over" synchronization the cache

Re: RFR JDK-8187653: Lock in CoderResult.Cache becomes performance bottleneck

2018-03-01 Thread Xueming Shen
On 03/01/2018 12:00 PM, Alan Bateman wrote: On 01/03/2018 19:42, Roger Riggs wrote: Hi Sherman, Looks to be a good fix with predictable behavior and performance. +1, Roger I assume malformed4 and unmappable4 should be final, the XXXCache fields too. -Alan Right, they all should be

Re: JDK-6372077: JarFile.getManifest() should handle manifest attribute names up to 70 bytes

2018-03-01 Thread Alan Bateman
On 01/03/2018 19:59, Roger Riggs wrote: Hi Alan, Thanks for the review. I added: "There must be a colon and a SPACE after the name; the combined length will not exceed 72 characters." Webrev updated in place: http://cr.openjdk.java.net/~rriggs/webrev-jar-6372077.patch/ Thanks, looks okay

Re: 8198888: Reduce string allocation churninInvokerBytecodeGenerator

2018-03-01 Thread Bernd Eckenfels
Cool, the lastClass varient then is pretty tempting (and I wont fix the multi-slot Version below, it misses a „lastUsedSlot“ field) Unrelated to this patch I feel that unifying internal names and external names will be a hard task, so Class#getInternalName() looks temting for a lot of similiar

Re: RFR: 8198888: Reduce string allocation churn in InvokerBytecodeGenerator

2018-03-01 Thread Paul Sandoz
> On Mar 1, 2018, at 4:14 AM, Claes Redestad wrote: > > Hi, > > two trivial optimizations that get rid of a few percent on ISC startup tests. > > Webrev: http://cr.openjdk.java.net/~redestad/819/jdk.00/ > Bug: https://bugs.openjdk.java.net/browse/JDK-819 >

Re: 8198888: Reduce string allocation churn inInvokerBytecodeGenerator

2018-03-01 Thread Claes Redestad
Hi Bernd, to dispel your concerns about keeping references to classes then InvokerBytecodeGenerators are shortlived and go out of scope after they've been used to generate an invoker (or LF) class, so the impl. here should be safe from a class leak perspective. A static cache is tempting,

Re: RFR: 8198888: Reduce string allocation churn in InvokerBytecodeGenerator

2018-03-01 Thread Paul Sandoz
It’s not just the internal VM name, it's also a descriptor in byte code. I hope the symbolic ref API currently being designed in the amber repo will help here. Until that settles i would be reluctant to consider adding public “toDescriptor” methods, or even do something internally right now if

Re: Oracle Java 8u161 regression in XML Schema Factory

2018-03-01 Thread Joe Wang
Hi Christoph and all, Just wanted to let you know that we're in progress to update the release notes for 6u181/7u171/8u161 with a doc for this change. Thanks, Joe On 2/22/2018 12:47 AM, Langer, Christoph wrote: Hi Joe, thanks for the clarification. It would be good to have a place of

Re: JDK-6372077: JarFile.getManifest() should handle manifest attribute names up to 70 bytes

2018-03-01 Thread Roger Riggs
Hi Alan, Thanks for the review. I added: "There must be a colon and a SPACE after the name; the combined length will not exceed 72 characters." Webrev updated in place:    http://cr.openjdk.java.net/~rriggs/webrev-jar-6372077.patch/ Thanks, Roger On 2/26/2018 6:50 AM, Alan Bateman wrote:

Re: JDK-6372077: JarFile.getManifest() should handle manifest attribute names up to 70 bytes

2018-03-01 Thread Xueming Shen
+1 On 03/01/2018 11:59 AM, Roger Riggs wrote: Hi Alan, Thanks for the review. I added: "There must be a colon and a SPACE after the name; the combined length will not exceed 72 characters." Webrev updated in place: http://cr.openjdk.java.net/~rriggs/webrev-jar-6372077.patch/ Thanks,

Re: RFR: JDK-8197594: String#repeat

2018-03-01 Thread Ivan Gerasimov
Hi Jim! I think the copying-loop can be slightly simplified, like this: for (; copied < limit - copied; copied <<= 1) { System.arraycopy(multiple, 0, multiple, copied, copied); } (didn't actually run it to check for correctness.) With kind regards, Ivan On 2/28/18 8:31 AM,

Re: RFR JDK-8187653: Lock in CoderResult.Cache becomes performance bottleneck

2018-03-01 Thread Alan Bateman
On 01/03/2018 20:10, Xueming Shen wrote: Right, they all should be final. webrev has been updated accordingly. Thanks, it looks okay now.

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-01 Thread Martin Buchholz
+ * @param codepoint a {@code codePoint}. does not match the formal parameter. javadoc -private should give an error on that.

Re: RFR: Here are some URLClassPath patches

2018-03-01 Thread Martin Buchholz
Ops. 8198810 is the id of the CSR, not the real bug - I should have used 8198803. So I may have broken a jira invariant. Probably jcheck should have caught my mistake. What now? changeset: 49117:0a93645a57f1 qparent user:martin date:2018-03-01 19:01 -0800 8198810:

Re: [JDK 11] RFR 8198821: fix test methods access for test java/text/Normalizer/NormalizerAPITest.java

2018-03-01 Thread Chris Yin
Thank you, Naoto Regards, Chris > On 2 Mar 2018, at 2:26 AM, Naoto Sato wrote: > > +1 > > Naoto > > On 2/28/18 6:03 PM, Chris Yin wrote: >> Please review the minor change for test >> java/text/Normalizer/NormalizerAPITest.java, thanks >> Added public access modifier

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

2018-03-01 Thread Lance Andersen
While running the JDK regression tests, I found a test that needed to be updated, test/langtools/tools/jdeps/modules/DotFileTest.java, due to the update to the java.sql module The updated webrev is at http://cr.openjdk.java.net/~lancea/8197533/webrev.03/

[11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-01 Thread naoto . sato
Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-4993841 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/4993841/webrev.03/ This stems from the recent discussion regarding String.repeat().[1] The corresponding CSR has already

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

2018-03-01 Thread Paul Sandoz
> On Mar 1, 2018, at 4:41 PM, Lance Andersen wrote: > > While running the JDK regression tests, I found a test that needed to be > updated, test/langtools/tools/jdeps/modules/DotFileTest.java, due to the > update to the java.sql module > > The updated webrev is at

Re: RFR JDK-8187653: Lock in CoderResult.Cache becomes performance bottleneck

2018-03-01 Thread Xueming Shen
On 3/1/18, 4:35 PM, David Holmes wrote: When you replace synchronized code with concurrent data structures you introduce race conditions that are precluded in the synchronized code. These need to be examined carefully to ensure they are safe. For example, whenever you replace a HashMap with a

Re: RFR: Here are some URLClassPath patches

2018-03-01 Thread Martin Buchholz
On Wed, Feb 28, 2018 at 3:58 PM, Martin Buchholz wrote: > > > 8198810: URLClassLoader does not specify behavior when URL array contains > null > http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassLoader-NPE/ > https://bugs.openjdk.java.net/browse/JDK-8198810 > Pushed.

Re: RFR JDK-8187653: Lock in CoderResult.Cache becomes performance bottleneck

2018-03-01 Thread David Holmes
Hi Sherman, On 24/02/2018 11:26 AM, Xueming Shen wrote: Hi, Please help review the proposed change to remove the potential performance bottleneck in CoderResult caused by the "over" synchronization the cache mechanism. issue: https://bugs.openjdk.java.net/browse/JDK-8187653 webrev:

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-01 Thread Martin Buchholz
The multiple redundant bounds checks bother me, but I don't know how to fix them without abandoning a bit of modularization. if (cp >= 0) { if (COMPACT_STRINGS && cp < 0x100) return latin1encode() if (cp < MIN_SURROGATE || (cp > MAX_SURROGATE && cp < MIN_SUPPLEMENTARY_CODEPOINT) return

Re: RFR: Here are some URLClassPath patches

2018-03-01 Thread Martin Buchholz
I have a patch to move those old jdi tests out of the ProblemList jail. I thought we had an existing bug in hotspot-svc to do this, but now I can't find it. We can just use well-formed URLs. http://cr.openjdk.java.net/~martin/webrevs/jdk/jdi-tests-malformed-urls/

Re: RFR: Here are some URLClassPath patches

2018-03-01 Thread David Holmes
On 2/03/2018 2:21 PM, Martin Buchholz wrote: Ops. 8198810 is the id of the CSR, not the real bug - I should have used 8198803. So I may have broken a jira invariant. Probably jcheck should have caught my mistake. What now? Now you have to manually update the real bug with the changeset

RE: Oracle Java 8u161 regression in XML Schema Factory

2018-03-01 Thread Langer, Christoph
Thanks, Joe. > -Original Message- > From: Joe Wang [mailto:huizhe.w...@oracle.com] > Sent: Donnerstag, 1. März 2018 20:11 > To: Langer, Christoph > Cc: OpenJDK Dev list > Subject: Re: Oracle Java 8u161 regression in XML Schema

Re: RFR: Here are some URLClassPath patches

2018-03-01 Thread David Holmes
On 2/03/2018 2:59 PM, Martin Buchholz wrote: I have a patch to move those old jdi tests out of the ProblemList jail. I thought we had an existing bug in hotspot-svc to do this, but now I can't find it. I've created a sub-task of 8198803 for this and assigned it to you.

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

2018-03-01 Thread Lance Andersen
> On Feb 28, 2018, at 8:29 PM, Paul Sandoz wrote: > > > >> On Feb 28, 2018, at 1:22 PM, Lance Andersen > > wrote: >> >>> >>> On Feb 28, 2018, at 2:20 PM, Lance Andersen >>

Re: RFR: JDK-8197594: String#repeat

2018-03-01 Thread Paul Sandoz
> On Mar 1, 2018, at 6:12 AM, Jim Laskey wrote: > > Peter, > > Very reasonable and worth considering. The main reason the power of 2 copy > works well is that the source is (almost) always cached. > That was my thinking too, the current approach is fine for

Re: RFR JDK-8197533 move javax.transaction.xa into its own module

2018-03-01 Thread Paul Sandoz
+1 > On Mar 1, 2018, at 8:59 AM, Lance Andersen wrote: >> >> +1, i second Joe’s request to update package-info.java while we are >> opportunistically cleaning this area up. > > Per your and Joe’s request, please see > cr.openjdk.java.net/~lancea/8197533/webrev.02

Re: [JDK 11] RFR 8198821: fix test methods access for test java/text/Normalizer/NormalizerAPITest.java

2018-03-01 Thread Naoto Sato
+1 Naoto On 2/28/18 6:03 PM, Chris Yin wrote: Please review the minor change for test java/text/Normalizer/NormalizerAPITest.java, thanks Added public access modifier to all “Test_" methods so they can be recognized as test method correctly by util class bug:

Re: RFR JDK-8187653: Lock in CoderResult.Cache becomes performance bottleneck

2018-03-01 Thread Alan Bateman
On 01/03/2018 19:42, Roger Riggs wrote: Hi Sherman, Looks to be a good fix with predictable behavior and performance. +1, Roger I assume malformed4 and unmappable4 should be final, the XXXCache fields too. -Alan