Hi again, Can some classlib guru review HARMONY-5791?
Thanks, Aleksey. On Fri, Apr 25, 2008 at 1:31 AM, Aleksey Shipilev <[EMAIL PROTECTED]> wrote: > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
