You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-dev@xmlgraphics.apache.org by Vincent Hennebert <vi...@anyware-tech.com> on 2007/07/16 16:38:29 UTC
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=rev&rev=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
Posted by Andreas L Delmelle <a_...@pandora.be>.
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
Posted by Andreas L Delmelle <a_...@pandora.be>.
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
Posted by "Peter B. West" <li...@pbw.id.au>.
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.
>
>
>> 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? :-]
Ho, ho.
alt-design always cached _all_ the Integer instances it needed. Another
startling new idea. As Manuel points out, unnecessary in 1.5+, unless
you are using a lot of Integers outside the range -128<=x<=127
--
Peter B. West <http://cv.pbw.id.au/>
Folio <http://defoe.sourceforge.net/folio/>
Re: IntMap.java
Posted by Chris Bowditch <bo...@hotmail.com>.
J.Pietschmann wrote:
> Vincent Hennebert wrote:
>
>> Shall we launch a poll on fop-user about abandoning support for 1.4 and
>> require 1.5 as a minimum? :-]
>
>
> A poll: maybe. Abandoning 1.3: Not yet.
Did you mean 1.4 here? I thought we had all agreed to drop support for
1.3 now? I have long argued that we maintain support for it due to the
length of time it takes for more recent JDKs to become available for
ancient o/s which large organisations tend to use to run their batch
processes on. AFICT, 98% of all o/s can now support 1.4, but not yet 1.5
so we shouldn't drop support for 1.4 for a while yet but I think its
safe to drop 1.3 support.
<snip/>
Chris
Re: IntMap.java
Posted by "Peter B. West" <li...@pbw.id.au>.
J.Pietschmann wrote:
> Vincent Hennebert wrote:
>> Shall we launch a poll on fop-user about abandoning support for 1.4 and
>> require 1.5 as a minimum? :-]
>
> A poll: maybe. Abandoning 1.3: Not yet.
> If the usage of those hash maps is only in a few places, we could
> provide JDK dependent code and tell people that FOP runs faster
> on JDK 1.5.
>
> J.Pietschmann
FOP should run markedly faster on 1.6 without any changes. Just about
everything else does.
--
Peter B. West <http://cv.pbw.id.au/>
Folio <http://defoe.sourceforge.net/folio/>
Re: IntMap.java
Posted by "J.Pietschmann" <j3...@yahoo.de>.
Vincent Hennebert wrote:
> Shall we launch a poll on fop-user about abandoning support for 1.4 and
> require 1.5 as a minimum? :-]
A poll: maybe. Abandoning 1.3: Not yet.
If the usage of those hash maps is only in a few places, we could
provide JDK dependent code and tell people that FOP runs faster
on JDK 1.5.
J.Pietschmann
Re: svn commit: r556112 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop:
fonts/truetype/TTFFile.java util/IntMap.java
Posted by "Peter B. West" <li...@pbw.id.au>.
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
Posted by Vincent Hennebert <vi...@anyware-tech.com>.
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
Posted by Manuel Mall <ma...@apache.org>.
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
Posted by Andreas L Delmelle <a_...@pandora.be>.
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
Posted by "J.Pietschmann" <j3...@yahoo.de>.
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