September 22, 2020 ☼ Pharo
Part I describes the new Pragma cache. And it noted that if we now use the “Pragma all” in #allSystemPragmas of PragmaCollector, everything should be faster (especially the menus in the editor, the use case that started the whole effort). So let’s do that:
But, re-doing the benchmark in a really large (that is, REALLY large) image, it turned out that it was actually not faster (due to too many pragmas).
(The fact that this benchmark was done after merging Cleanup-selectorsAndMethodsDo which is discussed below might have played a role, too…)
The solution was kind of obvious: the idea that PragmaCollector has an API where it is only possible to specify a filter that checks all pragmas is not optimal. It forces the collector to iterate over all Pragmas, which will always be slow. But all users of PragmaCollector are actually interested in specific pragmas and might filter them in addition (e.g. to check if the method has one argument). Thus:
This now solved the problem of slow menus, even in the large image. So DONE.
No! If you add/improve an API or speed something up substantially, there is quite some chance that the problem lead already to many, many workarounds.
This is especially true for system level changes: People often just “build on top”, they seldomly understand that improving the base system is even an option. This strangely even applies to the people developing the system itself, who you think should understand better…
So let’s analyse it for the case of the Pragmas and improve the whole system!
The missing #allNamed: was a reason for people to use PragmaCollector everywhere without needing it. What is bad with that? It is fairly slow (registering to system notifcation, raising Announcements…).
So we can replace all the uses of PragmaCollector where the instance is not retained by just using #allNamed:
While doing this and of course reading lots of code… some other simple cleanups got obvious. #withPragmasIn:do: was using #selectorsAndMethodsDo: without referencing the selector in the block. But #methodsDo: is faster! While at it, fix all the other users of #selectorsAndMethodsDo: systemwide that do the same.
Then there was a #pragmaDo: method on AdditionalMethodState with a bad name and no API on CompiledMethod. Add that, rename to #pragmasDo: and use it to speed up #withPragmasIn:do: even more.
Ah, and #pragmas on CompiledMethod could just use #streamContents: instead of implementing that itself:
Then we can use the new fast PragmaCollector API in the setting browser, speeding it up for large images:
Now if we look again at the PragmaMenuBuilder (which started all this)… we do actually not need the PragmaCollector instance at all!
With the improved PragmaMenuBuilder, there is no need for WorldState to keep the menuBuilder instance around:
KMPragmaKeymapBuilder does not need to have a pragme collector instance, either:
Not having long-running instances of PragmaCollector has another interesting impact: the instance registers to get an announcement on every method change. (while interestingly, the clients who really were interested did that, too, leading to two registrations for the same thing…)
Thus: these cleanups speed up even compilation and code loading!
BlockMenuBuilder was just added because getting the menu for Windows was too slow. We can remove it now:
In Cleanup-selectorsAndMethodsDo, we optimized the private method #withPragmasIn:do:. It actually has some senders outside, which tells us that here, too, an API is missing. “Give me all pragmas of that class”. A much better idea than to hand the class over to Pragma would be to ask the class itself. The same as we ask “Object methods” for all methods, we should be able to ask “Object pragmas”. #pragmasDo: is the optimized form to use for iterating without creating intermediate collections:
Here, too, you see that not just the new API was added, but all current users where fixed to use it. For other users, #withPragmasIn:do: was changed to automatically transform these the calling code to use #pragmasDo: instead.
And there is another very trivial cleanup from the backlog that I did while reading code. It just transforms one method to get rid of a code critics warning:
That was fun!