Re: svn commit: r556112 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fonts/truetype/TTFFile.java util/IntMap.java
Hi, Manuel Mall a écrit : On Wednesday 18 July 2007 02:58, Andreas L Delmelle wrote: snip/ Interestingly Java 1.5 has added the Integer.valueOf(int) method with the following comment: Flyweight pattern. That's what I was looking for before replying to Andreas' commit, and I was surprised to not find it in the 1.4 standard library. A good thing it finally got added. Returns a Integer instance representing the specified int value. If a new Integer instance is not required, this method should generally be used in preference to the constructor Integer(int), as this method is likely to yield significantly better space and time performance by caching frequently requested values. Obviously we can't use it because of backwards compatibility with 1.4 but it seems to address, to some extent, the performance issue you tried to solve. Indeed. And with the auto-boxing of primitive types we wouldn't even see those ugly new Integer(...) anymore. And guess what? The compiler makes use of Integer.valueOf() for auto-boxing. Shall we launch a poll on fop-user about abandoning support for 1.4 and require 1.5 as a minimum? :-] Vincent PS: BTW, thanks for your understanding, Andreas!
Re: svn commit: r556112 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fonts/truetype/TTFFile.java util/IntMap.java
Vincent Hennebert wrote: Hi, Manuel Mall a écrit : On Wednesday 18 July 2007 02:58, Andreas L Delmelle wrote: snip/ Interestingly Java 1.5 has added the Integer.valueOf(int) method with the following comment: Flyweight pattern. That's what I was looking for before replying to Andreas' commit, and I was surprised to not find it in the 1.4 standard library. A good thing it finally got added. Not a pattern. It's an object cache. -- Peter B. West http://cv.pbw.id.au/ Folio http://defoe.sourceforge.net/folio/
Re: svn commit: r556112 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fonts/truetype/TTFFile.java util/IntMap.java
On Jul 18, 2007, at 14:28, Peter B. West wrote: Hi Peter alt-design always cached _all_ the Integer instances it needed. Another startling new idea. FWIW, I did not presume my idea to be startling or new. Just was a bit bugged by the number of places in the current trunk where Integers are used, solely for the purpose of being able to store ints in a Map... Personally, I would not even cache Integer instances, but deal only with the primitives unless there was a really /compelling/ reason not to do so, not merely because 'It's cool to use Objects'. Following that latter motto, I'd as soon compile a new object type that is dedicated to storing primitive int-to-int mappings and avoid the construction of Integers altogether. Oh well, in the end more a question of personal taste, I guess. Cheers Andreas
Re: svn commit: r556112 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fonts/truetype/TTFFile.java util/IntMap.java
On Jul 16, 2007, at 22:25, J.Pietschmann wrote: Vincent Hennebert wrote: Addition of a general-purpose int-to-int map ... ... This change makes me a bit uneasy. No doubt that this class is clever and efficient and working, but that's something more to maintain. Jakarta Commons Collections has all kind of clever implementation. Don't they have already something similar, and if not, would it make sense to donate this implementation to Collections? Not sure if it would fit in there. It seems more meant for classes extending the Java Collections Framework. I precisely abandoned any attempt to make it fit into the JCF. Started out by implementing java.util.Map, but soon experienced that as adding additional annoyances than relieving any. As to the efficiency: I did some measurements of the difference in processing speed (for 64K elements), and the factor lies somewhere between 5 and 20 (times faster). A difference that is not caused by the HashMap lookups, but almost solely due to the necessary Integer constructions and casts... So, the motives were more aesthetic, I guess. More caused by my aversion for the dumb Integer immutables... I have nothing against HashMaps, but I just do not care much for lines of code like this: int someInt = ((Integer) someMap.get(new Integer (anotherInt))).intValue(); Granted, CPUs have become so fast and JVMs have been optimized in such a way that one does not even notice the difference unless when executing this ugly line of code 10 million times in a row, but still... Cheers Andreas
Re: svn commit: r556112 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fonts/truetype/TTFFile.java util/IntMap.java
On Wednesday 18 July 2007 02:58, Andreas L Delmelle wrote: On Jul 16, 2007, at 22:25, J.Pietschmann wrote: Vincent Hennebert wrote: Addition of a general-purpose int-to-int map ... ... As to the efficiency: I did some measurements of the difference in processing speed (for 64K elements), and the factor lies somewhere between 5 and 20 (times faster). A difference that is not caused by the HashMap lookups, but almost solely due to the necessary Integer constructions and casts... So, the motives were more aesthetic, I guess. More caused by my aversion for the dumb Integer immutables... I have nothing against HashMaps, but I just do not care much for lines of code like this: int someInt = ((Integer) someMap.get(new Integer (anotherInt))).intValue(); Granted, CPUs have become so fast and JVMs have been optimized in such a way that one does not even notice the difference unless when executing this ugly line of code 10 million times in a row, but still... Interestingly Java 1.5 has added the Integer.valueOf(int) method with the following comment: Returns a Integer instance representing the specified int value. If a new Integer instance is not required, this method should generally be used in preference to the constructor Integer(int), as this method is likely to yield significantly better space and time performance by caching frequently requested values. Obviously we can't use it because of backwards compatibility with 1.4 but it seems to address, to some extent, the performance issue you tried to solve. Cheers Andreas Cheers Manuel
Re: svn commit: r556112 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fonts/truetype/TTFFile.java util/IntMap.java
Hi Andreas, Author: adelmelle Date: Fri Jul 13 12:21:03 2007 New Revision: 556112 URL: http://svn.apache.org/viewvc?view=revrev=556112 Log: Addition of a general-purpose int-to-int map to replace Integer-to-Integer HashMaps + first usage in TTFFile Added: xmlgraphics/fop/trunk/src/java/org/apache/fop/util/IntMap.java (with props) This change makes me a bit uneasy. No doubt that this class is clever and efficient and working, but that's something more to maintain. By quickly looking at it I couldn't figure out exactly how it is working, and this is the kind of code that is very easy to modify in a wrong way. Granted, it's done, working, and it will probably not change much. But, well, even if a bit inconvenient and probably not the most efficient, hashmaps were basically doing the thing. What I mean is that, well, there are already so many other things to do... Moreover, before an IntMap makes the difference compared to a HashMap of Integers regarding performance, there are somewhat bigger architectural changes to perform ;-) Ok, I guess the fun you had while writing this class was a big motivation for the change ;-) Still. Just a thought, Vincent
Re: svn commit: r556112 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fonts/truetype/TTFFile.java util/IntMap.java
On Jul 16, 2007, at 16:38, Vincent Hennebert wrote: Hi This change makes me a bit uneasy. No doubt that this class is clever and efficient and working, but that's something more to maintain. By quickly looking at it I couldn't figure out exactly how it is working, and this is the kind of code that is very easy to modify in a wrong way. You're probably right about that. Should've made this more of a proposal than just going ahead. Granted, it's done, working, and it will probably not change much. But, well, even if a bit inconvenient and probably not the most efficient, hashmaps were basically doing the thing. Well, that was the thought. Not so much the HashMaps themselves, but more the look of all the Integers being constructed solely for the purpose of being able to store ints in a Map. What I mean is that, well, there are already so many other things to do... Moreover, before an IntMap makes the difference compared to a HashMap of Integers regarding performance, there are somewhat bigger architectural changes to perform ;-) That certainly is a fact! I just had this still lying around, noticed it while going over a unified diff, and decided to commit it, but it's probably best to wait a while, as I was actually still unsure of how to integrity-test such a thing... :/ Ok, I guess the fun you had while writing this class was a big motivation for the change ;-) Still. Hmm, it was indeed fun :-) OTOH, the small binary search loop is basically copied from some other class in the codebase. For the rest, it's pretty much common sense... That said, you are absolutely correct in questioning this change. I guess the wisest course of action, release-wise, is to revert this change while it is still little. I'll keep it in the closet to revisit at a later time. I'll take care of this tomorrow. Cheers Andreas
Re: svn commit: r556112 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop: fonts/truetype/TTFFile.java util/IntMap.java
Vincent Hennebert wrote: Addition of a general-purpose int-to-int map ... ... This change makes me a bit uneasy. No doubt that this class is clever and efficient and working, but that's something more to maintain. Jakarta Commons Collections has all kind of clever implementation. Don't they have already something similar, and if not, would it make sense to donate this implementation to Collections? J.Pietschmann
