moosetechnology / Moose

MOOSE - Platform for software and data analysis.
https://moosetechnology.github.io/moose-wiki
MIT License
136 stars 34 forks source link

Using Athens fonts is dead slow in Roassal #971

Closed seandenigris closed 9 years ago

seandenigris commented 9 years ago

Originally reported on Google Code with ID 971

Try this:
[view := ROMondrianViewBuilder new.
view shape label fontSize: [:x | x ].
view nodes: (10 to: 100).
view open] timeToRun  

On my MacBookPro i7 2.6 GHz this takes 23s.

The actual reason is that LogicalFont>>widthOfString: looks up the real font when the
font is not yet cached. And this ends up looking for files on the disk.

What is even worse is that the widthOfString: is being called during the rendering
cycle, so the whole image gets slow.

Reported by tudor@tudorgirba.com on 2013-08-24 20:48:01

seandenigris commented 9 years ago
Name: Roassal-TudorGirba.648
Author: TudorGirba
Time: 24 August 2013, 10:48:31.666 pm
UUID: b797927f-c4f9-40c5-b14d-30ecfccab900
Ancestors: Roassal-TudorGirba.647

Issue 971:  Using Athens fonts is dead slow in Roassal

fixed by caching the font in ROAbstractLabel

Reported by tudor@tudorgirba.com on 2013-08-24 20:48:55

seandenigris commented 9 years ago
After the fix, the same code works in 5s. To be mentioned that 

[10 to: 100 do: [ :x |
    (LogicalFont familyName: 'Arial' pointSize: x)
        widthOfString: 'test' ]] timeToRun

takes a bit less than 5s on my machine. Thus, essentially, the time spent now on the
rendering is only due to the first lookup of the fonts (no extra lookup is performed).
This is reasonable and it will get better once we get a better caching at the Pharo
level.

The cached font is reset on modelChanged:

I think this is a good enough solution.

Reported by tudor@tudorgirba.com on 2013-08-24 20:52:06

seandenigris commented 9 years ago
Ok, I was too fast.

This does not work properly because when we zoom in or out, we want to refresh the
font to make it larger or smaller. To this end, we should probably add the cache in
the ROFontOrganizerAthens.

Reported by tudor@tudorgirba.com on 2013-08-24 21:03:35

seandenigris commented 9 years ago
After loading Athens, then
[|view|
view := ROMondrianViewBuilder new.
view shape label fontSize: [:x | x ].
view nodes: (10 to: 100).
view open]

timeToRun ---> 4767 msec

By cheating...
LogicalFont>>height
    ^200
LogicalFont>>width
    ^200

then timeToRun ---> 45msec.

With...
LogicalFont>>height
    self realFont "height".
    ^200.

then timeToRun ---> 1929msec.

If I revert the changes above and then do...
LogicalFont>>realFont    
    "Smalltalk at: #btcHack put: Bag new"
    ^realFont ifNil: [
        (Smalltalk at: #btcHack) add: #count.
        realFont := self findRealFont]   ^

then timeToRun --> 4076
and (Smalltalk at: #btcHack) occurrencesOf: #count ---> 587 times it runs 
#findRealFont.

This might not be the proper place to do caching, but if change this to...
LogicalFont>>realFont
    | cache  |       
    cache := Smalltalk at: #btcHack2 ifAbsentPut: Dictionary new.
    ^realFont ifNil: [ cache at: self printString ifAbsentPut: [ self 
findRealFont ] ]

then timeToRun --> 1482msec.

subsequent runs have timeToRun --->  65msec.

Reported by benjamin.t.coman on 2013-08-25 01:37:07

seandenigris commented 9 years ago
> when we zoom in or out, we want to refresh the font to make it larger or smaller.

If you zoom through 100 unloaded point sizes, then it will likely be as slow.  I can
only think something convoluted like bitmap scaling the font during the zoom would
work.

Would you not keep the same font and Athens handle zooming?

Reported by benjamin.t.coman on 2013-08-25 02:08:22

seandenigris commented 9 years ago
btw, it seems that ROFontOrganizerAthens is not used, see attached.

Reported by benjamin.t.coman on 2013-08-25 02:15:54


seandenigris commented 9 years ago
That is because you did not install Athens :). It's not enough to load Athens in the
image. You also need to tell Roassal to use Athens.

MooseImageSetupCommandLineHandler>>#installAthens
    Gofer new
            smalltalkhubUser: 'Pharo' project: 'Athens';
            package: 'ConfigurationOfAthens';
            load.
    (Smalltalk at: #ConfigurationOfAthens) loadVersion: '2.1'.
    Gofer new
            smalltalkhubUser: 'ObjectProfile' project: 'Roassal';
            package: 'RoassalAthens';
            load.
    ROPlatform setCurrent: 'athens'.

Reported by tudor@tudorgirba.com on 2013-08-25 07:39:29

seandenigris commented 9 years ago
Ok, I moved the cache to ROFontsOrganizerAthens and it seems to work reasonably fine.

Name: RoassalAthens-TudorGirba.8
Author: TudorGirba
Time: 25 August 2013, 10:01:01.158 am
UUID: c0708bdf-583f-47fa-b622-aeaf329939a4
Ancestors: RoassalAthens-AlexandreBergel.7

added a defaultFonts cache to ROFontsOrganizerAthens to lazily cache the default fonts
per size. While the first time a certain size is used can still be slow, subsequent
calls are fast. This makes rendering significantly faster.

However, zooming in the presence of large amounts of fonts can still be slow.

Also, we now use the "StandardFonts defaultFont fontFamily" as the default font family
instead of the hardcoded Arial.

Reported by tudor@tudorgirba.com on 2013-08-25 08:01:41

seandenigris commented 9 years ago
> That is because you did not install Athens :). It's not enough to load Athens in the
image. You also need to tell Roassal to use Athens.

No, :) I did listen the first time, but...

ROPlatform current   ---> a ROPharoAthensPlatform
ROFontOrganizer current  ---> ROFontOrganizerMorphic

Reported by benjamin.t.coman on 2013-08-25 10:16:32

seandenigris commented 9 years ago
Ah, as far as I see, this mechanism is not used.

This is what gets used in ROAbstractLabel:

ROPlatform current fontOrganizerClass.

Actually, there are too many singletons in Roassal :)

Reported by tudor@tudorgirba.com on 2013-08-25 10:40:09

seandenigris commented 9 years ago
Hi Doru,

What is the status on that issue?
It is marked as fixed but I am not sure to have understood the fix for this

Reported by alexandre.bergel on 2013-08-29 13:40:55

seandenigris commented 9 years ago
Ah sorry, I missed  RoassalAthens-TudorGirba.8
Ok now :-)

Reported by alexandre.bergel on 2013-08-29 13:43:09