harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ilya Berezhniuk" <ilya.berezhn...@gmail.com>
Subject Re: [classlib][luni] TreeMap woes
Date Sat, 08 Dec 2007 21:30:06 GMT
If this bug is not in Eclipse, but in EUT, I suggest keeping TreeMap patches.

We already have several bugs filed on EUT. Significant part of these
bugs is caused by incorrect checks/comparisons relying on Sun
behavior.

So if this bug affects only incorrect tests and does not affect key
applications, we should keep our implementation, I think.

Thanks,
Ilya.

2007/12/9, Alexey Petrenko <alexey.a.petrenko@gmail.com>:
> 2007/12/9, Tim Ellison <t.p.ellison@gmail.com>:
> > Sergey Kuksenko wrote:
> > > I really found a bug. But I wish to note, that it is a bug in Eclipse.
> > > The story:
> > > In order to caught that I've created a "mixed" TreeMap. The mixed tree map
> > > aggregates two TreeMaps - old implementation and new implementation. Also
> > > mixed TreeMap performs all operation on both maps and compared results.
> > > Using that I've cought the place where differentce occures.
> > > It was operation remove originally from TreeSet.
> > > Let see.
> > > Before we have two identical maps:
> > >
> > > Old -  { 43731146; 91780893; 92440425; }
> > > New - { 43731146; 91780893; 92440425; }
> > >
> > > Numbers are identityHashCode values.
> > > Element "91780893" comes to remove.
> > > After remove we got two different maps:
> > >
> > > Old -  { 43731146; 92440425; }
> > > New - { 43731146; 91780893; 92440425; }
> > >
> > >  If we have (before remove) elements in this order(for both threes) - it
> > > means that elements has the following order  43731146 < 91780893 < 92440425
> > > BUT! I found that Eclipse's comparator used here changed the behaviour.
> > > According to my logs,  sign(cmp(43731146,91780893)) == 1 that means that
> > > 43731146 > 91780893
> > >
> > > If we compare "key to remove" with all element we got the follwoing signs:
> > > -1,0,-1. This is a error.
> >
> > I've seen a number of cases where applications do not implement to the
> > spec properly, and comparators seems to be a common case for mistakes
> > for some reason.
> >
> > > Why RI doesn't fail at this case. It's happiness. Because of on RI 91780893
> > > (key to remove) is on root and we compare key with the root first, got
> > > equality and all ok. Occaisionally.
> > > On my implementation we do the comparison 91780893 with 43731146(first
> > > element), got the fact that "key to remove" is less then smallest element in
> > > Tree and we have nothing to remove.
> >
> > This kind of behavior difference in the face of misbehaving application
> > is particularly difficult to match.
> >
> > > So I think we shouldn't rollback the pacth, but it makes sence to find a
> > > error in Eclipse and fill a bug against them.
> >
> > I definitely think it is worth filing a bug against Eclipse.
> >
> > Now what to do about the Harmony enhancement that causes Eclipse to
> > fail?  Do we really want to break for all the versions of Eclipse before
> > this bug of theirs is fixed?
> I would not say that we will break Eclipse. We'll just break three
> incorrect tests from Eclipse test suite.
> And unfortunately these tests are not the only tests failing in EUT on
> Harmony...
>
> SY, Alexey
>
> >
> > > Note, as wrote before, I found one bug in TreeMap. But, the can't impact on
> > > Eclipse unit tests, because of according my logs all trees in EUT smaller
> > > then 100. But the bug can took a place with small probability for trees more
> > > then 64*3 elemens and can't be for trees with size smaller then 3*64.
> >
> > I see Alexey patched that already.
> >
> > Regards,
> > Tim
> >
>

Mime
View raw message