[Pharo-project] Fwd: Adding FasterSets to Pharo: update

Oct. 31, 2009


      Thanks. I push the discussion to pharo since lot of people are  
interested.

Stef

Begin forwarded message:
...
From: Ralph Boland <rpboland@gmail.com>
Date: October 29, 2009 11:43:04 PM GMT+01:00
To: StÃ©phane Ducasse <stephane.ducasse@inria.fr>
Cc: Levente Uzonyi <leves@elte.hu>
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 can post this on the Pharo newsgroup if you like but perhaps you
should get Levente's
opinion first.
Regards
Ralph Boland

StÃ©phane Ducasse

Nicolas Cellier

tags

participants (2)