Mark, That's great there are no regressions on Commons-collections tests! BTW, can we adopt them as the part of BTI or luni tests?
I had created HARMONY-5791 for HashMap cleanup, and there's a first patch already, can you please take a look? I had extracted the contract-related methods there, so the change to IdentityHashMap should be pretty straightforward. After we finish with this issue, I could provide the clean script/patch for IdentityHashMap changes. Thanks, Aleksey. On Thu, Apr 24, 2008 at 11:31 PM, Mark Hindess <[EMAIL PROTECTED]> wrote: > +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 <[EMAIL PROTECTED]>, > > > "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 > > <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>: > > > > > > > > > > > > > > On 23 April 2008 at 0:36, "Aleksey Shipilev" <[EMAIL PROTECTED] > > > > > > > wrote: > > > > > Hi Endre, > > > > > > > > > > On Tue, Apr 22, 2008 at 11:30 PM, Endre St=F8lsvik <[EMAIL > PROTECTED] > > > 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. > > > > > > > > > > > > > > > > > > > >
