Rendering time of Point Morph

Bug #1761420 reported by hilaire on 2018-04-05
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Dr. Geo II
Medium
Unassigned

Bug Description

PointMorph are initialized with an empty label (text morph).
When a sketch comes with a lot of point, this emoty lable still slow down a lot the Athens canvas rendering.
Solution: keep the textmorph as nil as long as there is not label attached to a point.

Benoît St-Jean (bstjean) wrote :
Download full text (3.4 KiB)

I played with the "Approximation of Pi" to explore a bit of Dr. Geo's code and found a few things. I tested with 2000 points and different preferences/options.

First thing, the TextMorphs. Another way of doing it would be to lazy initialize #textMorph. With the "Approximation of Pi" script right now, setting the textMorph string to an empty string accounts for 18.9% of the processing time, let alone all the TextMorphs it adds to the heap and that have to be refreshed (for nothing!). The current script, for 2000 points, created 2000 TextMorphs that end up in the UI notification list for... nothing.

Second problem I noticed is the slowness of #doOneCycle.

When the application has to deal/refresh tons of morphs, the rendering heap quickly accumulates 4000+ morphs (I set the limit to 2000 points to profile it). If we'd have a #partialDoOneCycle (for lack of a better name), it would greatly speed things up! It could be used in cases where simulations generate a huge number of points.

I was able to get a 2x speedup by changing the script (last line before the closing bracket of the block, replacing "World doOneCycle" to:

(try \\ 10) = 0 ifTrue: [World doOneCycle].

We could keep track of #doOneCycle sends in a class variable somewhere in Geo and only refresh a certain percentage of the time (e.g. every 10 calls). Most of the time, we wouldn't need that but in cases where you generate *tons* of objects (usually Points), it would dramatically speed up things.

Another thing is the initialization of DrGPointCostumeStyle instances. The fact that the method sends 3 messages (setters) with 3 "self changed" consumes an awful lot of time (heap list has 4000 objects for 2000 points!). DrGPointCostumeStyle could be optimized in the #initialize method by creating a #color:pointSize:shape setter that would send a "self changed" only once instead of 3 times (as is the case right now). I have seen other cases similar to this one in the code.

Also, for some reason, the shape of the points also have an impact (I will investigate this later). My tests show a dramatic difference between circle and cross points as compared to square points, a ration of 2 to 1 !

here are the results I have collected (based on 4 tries each):

"Round point"
Milliseconds to execute script: 210455
World step list size: 4004

"Square point"
Milliseconds to execute script: 104065
World step list size: 4004

"Round point, refreshed every 10 times"
Milliseconds to execute script: 117344
World step list size: 4004

"Square point, refreshed every 10 times"
Milliseconds to execute script: 51305
World step list size: 4004

"Round point, DrGPointCostume with TextMorph initialized to nil"
Milliseconds to execute script: 115285
World step list size: 2004

"Cross point, DrGPointCostume with TextMorph initialized to nil"
Milliseconds to execute script: 114876
World step list size: 2004

"Square point, DrGPointCostume with TextMorph initialized to nil"
Milliseconds to execute script: 46316
World step list size: 2004

"Round point, DrGPointCostume with TextMorph initialized to nil, refreshed every 10 times"
Milliseconds to execute script: 49098
World step list size: 2004

"Cross point,...

Read more...

Nice analysis!

Yep, the empty text morph label of point is very expensive. It was
already identified, and fixed.

Regarding your idea about partial World update, it is neat. Dedicated
methods should be implemented on the DrGeoCanvas class; it is really the
one knowing about the state of the canvas.

We can have several messages:

#updateCanvasFrequency: integer

#updateCanvas

Two additional attributes for DrGeoCanvas are needed,
updateCanvasFrequency and updateCanvasCounter

Round point are very expensive to render. The Cairo backend needs it to
be render as arc. It could be optimized in the DrGPointMoprh with a
cache Form. But I don't want to complicate its code. Now from Smalltalk
Sketch, point are rendered by default as squares, which is likely even
faster than cross.

About the point costume style, the setters need to be update
individually too. For example the #color: message needs its own #changed
message sent when the user edit its style from the GUI. How will you
write #color:pointSize:shape setter and avoid code duplication at the
same time? Writing private #color, #pointSize: #shape setters without
the changed message. I am not sure what is the best practice? In the
other hand, in the PI script, costume is initialized once per world
update cycle, when a new the point is created. I am not sure it is a
problem, is it? I have not measure its impact.

Good points!

Thanks

Hilaire

--
GNU Dr. Geo
http://drgeo.eu

Changed in drgeo:
status: Confirmed → In Progress
Changed in drgeo:
milestone: 18.06 → wip
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers