Hi Martin, this also looks like a good catch to me, +1.
When you push, could you insert the copyright block from, e.g. src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XML11DocumentScannerImpl.java at the place where you currently find the " reserved comment block ", first 4 lines? The XML copyright headers get cleaned up as they are touched... Thanks and best regards Christoph > -----Original Message----- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf > Of Martin Buchholz > Sent: Samstag, 1. Oktober 2016 18:55 > To: Claes Redestad <claes.redes...@oracle.com> > Cc: core-libs-dev <core-libs-dev@openjdk.java.net> > Subject: Re: RFR: 8167002: JAXP schema validator: Use HashSet instead of > ArrayList for tracking XML IDs > > On Sat, Oct 1, 2016 at 5:45 AM, Claes Redestad <claes.redes...@oracle.com> > wrote: > > > > > On 2016-10-01 02:41, Martin Buchholz wrote: > > > >> https://bugs.openjdk.java.net/browse/JDK-8167002 > >> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/xml-id-v > >> alidation-by-hash/ > >> > >> > > +1, but I have to ask what the intended benefit of writing: > > > > ((fIds != null) ? fIds : (fIds = new HashSet<>())).add(name); > > > > rather than keeping the pre-existing pattern: > > > > if (fIds == null) fIds = new HashSet<>(); > > fIds.add(name); > > > > If this is about bytecode optimization to help with inlining or such, > > the latter actually generate a more compact method (14 vs 16 bytecodes). > > > The intent was (by instinct) to not code a re-read of a field, but ... yeah > .... that wasn't achieved. > That could be done correctly using > > public void addId(String name) { > HashSet<String> x; > if ((x = fIds) == null) fIds = x = new HashSet<>(); > x.add(name); > } > > but I'll just revert to > if (fIds == null) fIds = new HashSet<>(); > fIds.add(name);
