I don't think this discussion is really related to my change introduced in r1748015. So I leave the optimization topics to our JIT wizards ;-)
Benedikt James Carman <ja...@carmanconsulting.com> schrieb am Mo., 13. Juni 2016 um 00:02 Uhr: > That's still a tough one (breakfast). Crazy weekend. > > It looks like the switch (which to me is more readable) does a hashCode() > call first to find the right place to jump to and then does equals. So, > theoretically it would be faster, especially for large numbers of cases > (unless the compiler is smart enough to recognize when it's effectively a > switch implemented as ifs). It would be interesting to see how well it get > optimized away at runtime though (if I wasn't on vacation tomorrow I might > play with that a bit). It would also be interesting to see the compiled > bytecode side-by-side for the Sun compiler. > On Sun, Jun 12, 2016 at 5:49 PM Gary Gregory <garydgreg...@gmail.com> > wrote: > > > On Jun 12, 2016 2:45 PM, "James Carman" <ja...@carmanconsulting.com> > > wrote: > > > > > > I don't off the top of my head. All I have is anecdotal stuff in my > > brain, > > > but then again I can't remember what I had for lunch yesterday either. > :) > > > > How about breakfast this morning? ;-) > > > > Gary > > > > > > On Sun, Jun 12, 2016 at 5:42 PM dbrosIus <dbros...@baybroadband.net> > > wrote: > > > > > > > Sure I got that. I thought you had a source that explains an if/else > > > > chain getting jitted down to a simple hash jumptable > > > > > > > > -------- Original message -------- > > > > From: James Carman <ja...@carmanconsulting.com> > > > > Date: 06/12/2016 5:19 PM (GMT-05:00) > > > > To: Commons Developers List <dev@commons.apache.org> > > > > Subject: Re: svn commit: r1748015 - in > > /commons/proper/imaging/trunk/src: > > > > changes/changes.xml > > > > main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java > > > > > > test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java > > > > > > > > I was referring to JIT. > > > > > > > > On Sun, Jun 12, 2016 at 5:18 PM dbrosIus <dbros...@baybroadband.net> > > > > wrote: > > > > > > > > > Interesting. Source? > > > > > > > > > > -------- Original message -------- > > > > > From: James Carman <ja...@carmanconsulting.com> > > > > > Date: 06/12/2016 5:12 PM (GMT-05:00) > > > > > To: Commons Developers List <dev@commons.apache.org> > > > > > Subject: Re: svn commit: r1748015 - in > > /commons/proper/imaging/trunk/src: > > > > > changes/changes.xml > > > > > > main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java > > > > > > > test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java > > > > > > > > > > Doesn't it all compiled down to the same thing? It gets optimized > at > > > > > runtime anyway. I would go for the more readable option, unless > there > > are > > > > > extremely compelling performance numbers. > > > > > > > > > > On Sun, Jun 12, 2016 at 4:38 PM Benedikt Ritter < > brit...@apache.org> > > > > > wrote: > > > > > > > > > > > Yes, the if-else could also be implemented using a switch > > statement. > > > > But > > > > > is > > > > > > that really more efficient? > > > > > > > > > > > > Benedikt > > > > > > > > > > > > Gary Gregory <garydgreg...@gmail.com> schrieb am So., 12. Juni > > 2016 > > um > > > > > > 22:05: > > > > > > > > > > > > > This looks like a candidate for the more efficient switch on > > string. > > > > > > > > > > > > > > Gary > > > > > > > > > > > > > > On Sun, Jun 12, 2016 at 7:47 AM, <brit...@apache.org> wrote: > > > > > > > > > > > > > > > Author: britter > > > > > > > > Date: Sun Jun 12 14:47:11 2016 > > > > > > > > New Revision: 1748015 > > > > > > > > > > > > > > > > URL: http://svn.apache.org/viewvc?rev=1748015&view=rev > > > > > > > > Log: > > > > > > > > IMAGING-178: PnmImageParser does not check the validity of > > input > > > > PAM > > > > > > > > header. Thanks to emopers. This also fixes #20 from github. > > > > > > > > > > > > > > > > Modified: > > > > > > > > commons/proper/imaging/trunk/src/changes/changes.xml > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java > > > > > > > > > > > > > > > > Modified: > commons/proper/imaging/trunk/src/changes/changes.xml > > > > > > > > URL: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/changes/changes.xml?rev=1748015&r1=1748014&r2=1748015&view=diff > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ============================================================================== > > > > > > > > --- commons/proper/imaging/trunk/src/changes/changes.xml > > (original) > > > > > > > > +++ commons/proper/imaging/trunk/src/changes/changes.xml Sun > > Jun 12 > > > > > > > > 14:47:11 2016 > > > > > > > > @@ -46,6 +46,9 @@ The <action> type attribute can be add,u > > > > > > > > <body> > > > > > > > > > > > > > > > > <release version="1.0" date="TBA" description="First > major > > > > > > release"> > > > > > > > > + <action issue="IMAGING-178" dev="britter" type="fix" > > > > > > > > due-to="emopers"> > > > > > > > > + PnmImageParser does not check the validity of input > > PAM > > > > > > header. > > > > > > > > + </action> > > > > > > > > <action issue="IMAGING-184" dev="ggregory" > > type="update"> > > > > > > > > Update platform from Java 5 to 7 > > > > > > > > </action> > > > > > > > > > > > > > > > > Modified: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java > > > > > > > > URL: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java?rev=1748015&r1=1748014&r2=1748015&view=diff > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ============================================================================== > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java > > > > > > > > (original) > > > > > > > > +++ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > commons/proper/imaging/trunk/src/main/java/org/apache/commons/imaging/formats/pnm/PnmImageParser.java > > > > > > > > Sun Jun 12 14:47:11 2016 > > > > > > > > @@ -157,18 +157,28 @@ public class PnmImageParser extends > Imag > > > > > > > > final String type = tokenizer.nextToken(); > > > > > > > > if ("WIDTH".equals(type)) { > > > > > > > > seenWidth = true; > > > > > > > > + if(!tokenizer.hasMoreTokens()) > > > > > > > > + throw new ImageReadException("PAM > > header > > > > has > > > > > > no > > > > > > > > WIDTH value"); > > > > > > > > width = > > > > Integer.parseInt(tokenizer.nextToken()); > > > > > > > > } else if ("HEIGHT".equals(type)) { > > > > > > > > seenHeight = true; > > > > > > > > + if(!tokenizer.hasMoreTokens()) > > > > > > > > + throw new ImageReadException("PAM > > header > > > > has > > > > > > no > > > > > > > > HEIGHT value"); > > > > > > > > height = > > > > > Integer.parseInt(tokenizer.nextToken()); > > > > > > > > } else if ("DEPTH".equals(type)) { > > > > > > > > seenDepth = true; > > > > > > > > + if(!tokenizer.hasMoreTokens()) > > > > > > > > + throw new ImageReadException("PAM > > header > > > > has > > > > > > no > > > > > > > > DEPTH value"); > > > > > > > > depth = > > > > Integer.parseInt(tokenizer.nextToken()); > > > > > > > > } else if ("MAXVAL".equals(type)) { > > > > > > > > seenMaxVal = true; > > > > > > > > + if(!tokenizer.hasMoreTokens()) > > > > > > > > + throw new ImageReadException("PAM > > header > > > > has > > > > > > no > > > > > > > > MAXVAL value"); > > > > > > > > maxVal = > > > > > Integer.parseInt(tokenizer.nextToken()); > > > > > > > > } else if ("TUPLTYPE".equals(type)) { > > > > > > > > seenTupleType = true; > > > > > > > > + if(!tokenizer.hasMoreTokens()) > > > > > > > > + throw new ImageReadException("PAM > > header > > > > has > > > > > > no > > > > > > > > TUPLTYPE value"); > > > > > > > > tupleType.append(tokenizer.nextToken()); > > > > > > > > } else if ("ENDHDR".equals(type)) { > > > > > > > > break; > > > > > > > > > > > > > > > > Modified: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java > > > > > > > > URL: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > http://svn.apache.org/viewvc/commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java?rev=1748015&r1=1748014&r2=1748015&view=diff > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ============================================================================== > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java > > > > > > > > (original) > > > > > > > > +++ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > commons/proper/imaging/trunk/src/test/java/org/apache/commons/imaging/formats/pnm/PnmImageParserTest.java > > > > > > > > Sun Jun 12 14:47:11 2016 > > > > > > > > @@ -46,4 +46,12 @@ public class PnmImageParserTest { > > > > > > > > PnmImageParser underTest = new PnmImageParser(); > > > > > > > > underTest.getImageInfo(bytes, params); > > > > > > > > } > > > > > > > > + > > > > > > > > + @Test(expected = ImageReadException.class) > > > > > > > > + public void testGetImageInfo_missingWidthValue() throws > > > > > > > > ImageReadException, IOException { > > > > > > > > + byte[] bytes = "P7\nWIDTH \n".getBytes(US_ASCII); > > > > > > > > + Map<String, Object> params = Collections.emptyMap(); > > > > > > > > + PnmImageParser underTest = new PnmImageParser(); > > > > > > > > + underTest.getImageInfo(bytes, params); > > > > > > > > + } > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > E-Mail: garydgreg...@gmail.com | ggreg...@apache.org > > > > > > > Java Persistence with Hibernate, Second Edition > > > > > > > <http://www.manning.com/bauer3/> > > > > > > > JUnit in Action, Second Edition < > > http://www.manning.com/tahchiev/> > > > > > > > Spring Batch in Action <http://www.manning.com/templier/> > > > > > > > Blog: http://garygregory.wordpress.com > > > > > > > Home: http://garygregory.com/ > > > > > > > Tweet! http://twitter.com/GaryGregory > > > > > > > > > > > > > > > > > > > > > > > > >
