2009/10/31 Stéphane Ducasse <[email protected]>: > Thanks. I push the discussion to pharo since lot of people are > interested. > > Stef > > Begin forwarded message: > >> From: Ralph Boland <[email protected]> >> Date: October 29, 2009 11:43:04 PM GMT+01:00 >> To: Stéphane Ducasse <[email protected]> >> Cc: Levente Uzonyi <[email protected]> >> Subject: Adding FasterSets to Pharo: update >> >> I took a look at FasterSets and Levente Uzonyi's version. >> I compared the two versions as of the first release of Levente's >> version to Squeak. Note that he has released additional >> changes since then. >> Here is my report. >> >> 1) Bugs: >> >> FasterSets has two problems: >> a) KeyedIdentitySet is missing method ScanForNil: >> b) PluggableSet is missing method NoCompareOrGrowAddAll: >> Both cases should cause failure. Note that I have been using >> this code for years without problem. I assume that is because >> the subset >> of Squeak that I used doesn't include using these classes. >> These problems are easily fixed. >> >> In Levente's code class WeakKeyToCollectionDictionary->rehash >> seems to be >> missing. This appears to be a mistate to me but I will live it >> to Levente to explain >> it. The fix, if needed, is simple. >> >> 2) Code quality: >> For me the code quality is about the same but Levente considers >> his version to be better. >> In any case changes in one version is easily ported to the other >> so it would be easy to >> perform any cleanup you might want. Three notes in particular. >> a) Levente's version of rehash is cleaner and should be used >> instead of mine. >> b) Levente's version does not use method scanForNilFrom: (He >> would probably call >> the method scanForEmptySlotFrom:). >> I don't know why he didn't do this: it clearly cleaner. >> >> 3 Performance. >> a) According to Levente's stats rehash is clearly faster in >> his version than mine. I see >> no reason no not prefer his version to mine. >> >> b) The improvement in performance for method add: in >> Levente's version >> is insignificant (appx 1%); this difference could easily >> be made up in minor >> changes to my code. Thus here I consider there to be no >> difference. >> >> b) Levente's version does not apply the FasterSets idea to >> MethodDictionary. >> I am not sure why he did not do this but it may have >> something to do with the fact that >> mistakes can easily corrupt your image. The version in >> FasterSets works fine though >> it took three tries for me to get it right (mostly because >> of carelessness). >> >> c) I left methods intersection and nastyAddNew: out of classes >> Set and Dictionary for >> the initial version of FasterSets to keep the number of >> changes to a minimum. >> At your request I added them to the current version. >> These methods are not in >> Levente's version. They are public methods that provide >> performance improvements. >> If desired they could easily be added to Levente's version. >> >> >> Based on the above I would say that there is not a great deal of >> difference between Levente's version and mine but what difference >> there is favors his version. >> >> I recommend that >> >> 1) You use Levente's version. >> 2) Sort out the issue of no rehash method for Class >> WeakKeyToCollectionDictionary. >> 3) Modify his code to use method scanForNilFrom: (calling it >> scanForEmptySlotFrom:). >> 4) Decide if you want to add methods intersect: and >> nastyAddNew:. >> I can add these if you want. >> 5) Release a version of Pharo with these changes. >> 6) In the following release of Pharo add the changes to >> MethodDictionary to use The >> FasterSets Idea. I sugggest doing this separately from >> everything else just because it is a >> bit hairy. I would not leave this change out because >> MethodDictionary is too important >> a class not to have these improvements. >> >> >> One final comment. If you really want to make the Set hierarchy >> better I would suggest that >> you move the Dictionary hierarchy out of the Set hierarchy >> altogether. This means some >> duplication of code but makes for a much cleaner implementation. For >> example you get to >> avoid the mess with MethodDictionary. Of the three implementations >> of Smalltalk that I >> have looked at Squeak/Pharo is the only version that uses one >> hierarchy. >> This is probably too major a change to make but at least you now know >> my opinion on the >> matter. >>
You dont have to duplicate. You can just derive the two classes from a common HashedCollection ancestor. http://lists.squeakfoundation.org/pipermail/squeak-dev/2007-June/117894.html Nicolas >> >> You can post this on the Pharo newsgroup if you like but perhaps you >> should get Levente's >> opinion first. >> >> Regards >> >> Ralph Boland > > > _______________________________________________ > Pharo-project mailing list > [email protected] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > _______________________________________________ Pharo-project mailing list [email protected] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
