[Cuis-dev] allSubclasses and withAllSubclasses improvement

[Hernan Wilkinson]

I'm having problems with the statistics...
I tried Andres' version of allSubclasses and to my surprise it is slower
than my version in the Stack VM, that is the one I use for Live Typing, but
it is slightly faster with the JIT VM (only a 0.01%)
I don't see the 20% improvement that you mentioned Andres, what are the
number you got?
These are the numbers I get doing this, three times:

Smalltalk garbageCollect.
Time millisecondsToRun: [ 1000 timesRepeat: [ Behavior allSubclasses ]].

JIT VM:
Breath first version:   169  170  171
Depth first version: 181 179 180

Stack VM:
Breath first version:  776  780  788
Depth first version: 749  749  751


On Sat, Aug 24, 2019 at 11:22 AM Juan Vuletich via Cuis-dev <
cuis-dev at lists.cuis.st> wrote:

> Uh, I hadn't seen you had already attached code, and only answered to
> the comments. Apologies for that!
>
> This new version is much faster. Thanks! I integrated it at GitHub, with
> a short comment about the iterative vs. recursive choice.
>
> Cheers,
>
> --
> Juan Vuletich
> www.cuis-smalltalk.org
> https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
> https://github.com/jvuletich
> https://www.linkedin.com/in/juan-vuletich-75611b3
> @JuanVuletich
>
>
>
> On 8/23/2019 7:34 PM, Andres Valloud via Cuis-dev wrote:
> > See attached for your reference.
> >
> > On 8/22/19 05:36, Juan Vuletich wrote:
> >> On 8/21/2019 4:29 PM, Andres Valloud via Cuis-dev wrote:
> >>> What do you see as "ugly"?
> >>
> >> It is way more complicated than needed to solve the problem. The
> >> problem is trivial, this code is not. And there is no reason for
> >> this: class hierarchy depths shouldn't be so deep to be a challenge
> >> for the call stack.
> >>
> >> Hernán's code is only as complex as the problem, without accidental
> >> complexity (in the Fred Brooks sense). It is a clear explanation of
> >> the problem and the solution.
> >>
> >> To me, code aesthetics is not only about code being clear, and a good
> >> explanation of the problem and the solution. It is also about being
> >> the best fit for the problem (including human readers here: the
> >> system is the curricula). Sometimes, performance requirements or
> >> space constraints require additional complexity. Then, I see that
> >> extra complexity as pretty. This is of course subjective, like any
> >> aesthetic judgment.
> >>
> >>> One detail I don't see mentioned here: the old code filled the
> >>> ordered collection breadth first (before the set conversion), and so
> >>> it has limited execution stack depth.  The current code fills the
> >>> collection depth first, so its execution stack depth is as deep as
> >>> the hierarchy tree.
> >>>
> >>> Maybe there was a reason why the users of allSubclasses were better
> >>> off with breadth first, before someone put in asSet.  Or perhaps
> >>> asSet was always there, I don't see why that was beneficial.
> >>>
> >>> In any case, I tried both the recursive and (properly implemented)
> >>> iterative methods: breadth first is >20% faster.
> >>>
> >>> On 8/21/19 05:44, Juan Vuletich via Cuis-dev wrote:
> >>>> Wow, thanks. The old code was slow and ugly!
> >>>>
> >>>> Integrating it.
> >>>>
> >>>> Cheers,
> >>>>
> >>>> --
> >>>> Juan Vuletich
> >>>> www.cuis-smalltalk.org
> >>>> https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
> >>>> https://github.com/jvuletich
> >>>> https://www.linkedin.com/in/juan-vuletich-75611b3
> >>>> @JuanVuletich
> >>>>
> >>>>
> >>>> On 8/21/2019 9:26 AM, Hernan Wilkinson via Cuis-dev wrote:
> >>>>> Hi,
> >>>>>  attached is a cs that changes the implementation of
> >>>>> #allSubclasses and #withAllSubclasses and generates performance
> >>>>> improvement drastically.
> >>>>>  Just as a quick example, with the current implementation:
> >>>>>  Time millisecondsToRun: [ 100 timesRepeat: [Behavior
> >>>>> withAllSubclasses ]] 1140 .
> >>>>>  With the proposed change:
> >>>>>  Time millisecondsToRun: [ 100 timesRepeat: [Behavior
> >>>>> withAllSubclasses2 ]]  . 73
> >>>>>
> >>>>>  In the type checker of LiveTyping, checking Behavior goes from
> >>>>> 9.5 seconds to 5 seconds only with that change.
> >>>>>  One difference between the current and proposed version, is that
> >>>>> the current returns a Set while the proposed version returns an
> >>>>> OrderedCollection.
> >>>>>  I looked carefully if that could break something and it does not.
> >>>>> I've been using the image with this new implementation and
> >>>>> everything works fine.
> >>>>>
> >>>>>  Juan, please take a look at it and integrate it if you think it
> >>>>> is useful.
> >>>>>  Hernan.
> >>>>>
> >>>>> --
> >>>>> *Hernán Wilkinson
> >>>>> Agile Software Development, Teaching & Coaching*
> >>>>> *Phone: +54-011*-4893-2057
> >>>>> *Twitter: @HernanWilkinson*
> >>>>> *site: http://www.10Pines.com <http://www.10pines.com/>*
> >>>>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
> >>>>
> >>
> >>
>
>
> --
> Cuis-dev mailing list
> Cuis-dev at lists.cuis.st
> https://lists.cuis.st/mailman/listinfo/cuis-dev
>


-- 

*Hernán WilkinsonAgile Software Development, Teaching & Coaching*
*Phone: +54-011*-4893-2057
*Twitter: @HernanWilkinson*
*site: http://www.10Pines.com <http://www.10pines.com/>*
Address: Alem 896, Floor 6, Buenos Aires, Argentina
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190824/2f0a11ee/attachment-0001.htm>