harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hindess <mark.hind...@googlemail.com>
Subject Re: [classlib][luni][performance] *HashMap cleanup and optimization (was: Re: [classlib][luni][performance] IdentityHashMap implementation)
Date Thu, 24 Apr 2008 19:31:28 GMT
+1

Incidentally, I tested your previous patch and it looks good to me.  I
also tried running the commons-collections tests - thinking that these
might be a good way to achieve better coverage - and they also pass
except for one unrelated failure (HARMONY-5788).

I've not had chance to look at the performance implications yet.

-Mark.

[0] http://commons.apache.org/collections/

In message <4bebff790804240845n10e307f3uffce5749accc849b@mail.gmail.com>,
"Aleksey Shipilev" writes:
>
> Thanks, Alexey!
> 
> I see the plan is following:
>  1. Sweep the HashMap implementation and make the source beatuful: add
> necessary comments, re-layout class members.
>  2. Test-commit-test sweeped HashMap implementation and see there are
> no breakages.
>  3. Remove legacy IdentityHashMap and copy HashMap over it (using svn
> capabilities)
>  4. Transform new IdentityHashMap to real IdentityHashMap (hashCode ->
> identityHashCode, equals -> == and stuff)
>  5. Test-commit-test new IdentityHashMap.
> 
> Then we could think about generalization of IdentityHashMap and HashMap code.
> 
> Nathan, Mark, Alexey, do I summarize correct?
> 
> Thanks,
> Aleksey.
> 
> On Wed, Apr 23, 2008 at 4:43 PM, Alexey Petrenko
> <alexey.a.petrenko@gmail.com> wrote:
> > Aleksey,
> >
> >  I would support Mark here. Clear patch is very important because it
> >  makes our lives much easier and we are not required to skip 70K of
> >  irrelevant text to see 2 relevant lines :)
> >  So I think that your idea is good but should be better delivered :)
> >
> >  Take a look at "Good Issue Resolution Guideline" for example.
> >  http://harmony.apache.org/issue_resolution_guideline.html
> >
> >  SY, Alexey
> >
> >  2008/4/23, Mark Hindess <mark.hindess@googlemail.com>:
> >
> >
> > >
> >  > On 23 April 2008 at 0:36, "Aleksey Shipilev" <aleksey.shipilev@gmail.com
> >
> >  > wrote:
> >  > > Hi Endre,
> >  > >
> >  > > On Tue, Apr 22, 2008 at 11:30 PM, Endre St=F8lsvik <Endre@stolsvik.com
> > wro=
> >  > > te:
> >  > > > Aleksey Shipilev wrote:
> >  > > > > The reason behind all that changes is that entire IdentityHashMap
> >  > > > > implementation was thrown away and replaced by HashMap
> >  > > >
> >  > > >  Isn't it possible to actually record this fact using SVN, by
> >  > > > deleting the file, and then adding it again (or svn copy it from
> >  > > > HashMap) - so that it doesn't look like a *change*, but more what
it
> >  > > > actually is: a remove, and then an add (actually, a copy)?
> >  > >
> >  > > Unfortunately, that's not usable, you might play around to see why. If
> >  > > you find a solution, please let me know :)
> >  >
> >  > Fortunately, it is not compulsory to create patches with "svn diff".
> >  > I've just done:
> >  >
> >  > 1) apply your patch to a fresh checkout
> >  > 2) cp modules/luni/src/main/java/java/util/HashMap.java \
> >  >      modules/luni/src/main/java/java/util/IdentityHashMap.java.orig
> >  > 3) diff -u modules/luni/src/main/java/java/util/IdentityHashMap.java.ori
> g \
> >  >           modules/luni/src/main/java/java/util/IdentityHashMap.java
> >  >
> >  > The resulting patch would be much more suitable for attachment to a JIRA
> .
> >  >
> >  > I'd still fix a few things about HashMap.java first though.
> >  > -Mark.
> >  >
> >  >
> >  >
> >
> 



Mime
View raw message