Making the Pharo settings browser open faster with GTInspector

In a recent post on the Pharo mailing list, Jannik noticed that when opening the System Browser on a single package settings, it is really slow. What could the problem be?

Let’s see. I actually do not know anything about the internal implementation of the browser, but we can start from the line that came with the original report:

SettingBrowser new setViewedPackageNames: 'Settings-Network'; open.

To investigate the problem, we use the GTInspector support for message tally.

Inspecting:

MessageTally new
     spyEvery: 1
     on: [
          SettingBrowser new
               setViewedPackageNames: 'Settings-Network';
               open ];
     yourself

reveals a 10s delay until opening the window. Indeed, something looks strange.

Investigating the execution further shows that the problem is related to applying filtering in updateAccordingTo:.

Going a step back, we notice that the filter is being applied only when viewedPackages is not empty. We get confident that we am on the right path.

At a deeper inspection, we see that PackageOrganizer gets involved. That rings a bell: Pharo 3.0 comes with the RPackage support exactly to fix the problems of PackageOrganizer.

Thus, one thing to do is to use RPackage. For this we need two things:

1. modify the lookup of the package, and
2. modify the setting of packages.

Armed with this knowledge, we can modify:

SettingNode>>receiverPackage
     ^ self settingReceiver
          ifNotNil: [self settingReceiver class package]

and:

SettingBrowser>>setViewedPackageNames: aText
     | allViewed |
     allViewed := Set new.
     aText asString substrings
          do: [:sub | (RPackageOrganizer default
                              packageNamed: sub
                              ifAbsent: [])
                    ifNotNil: [:pkg | allViewed add: pkg]].
     self changePackageSet: allViewed.
     self changed: #getViewedPackageNames

Did it solve the problem? Re-running the snippet after the modifications, gets us a 1.8s result:

Great! It’s probably not the best we could do, but it becomes reasonable. Yet, is the job ready?

Not quite. We only modified things that were related to the exercised use case. What if there are other parts in the implementation that depend on the old PackageOrganizer and related classes? Of course, we could run all the tests, but unfortunately there are no tests for this browser. Or we could just click around, but we would still not know if we clicked enough.

Or we could search for what we know already: all references to PackageOrganizer and PackageInfo should be replaced:

(RPackageOrganizer default packageNamed: 'System-Settings')
     methodReferences select: [:each |
          | ast |
          ast := each method parseTree.
          (ast references: PackageOrganizer name)
               or: [ ast  references: PackageInfo name ] ]

This gets us one method:

SettingNode>>package

     ^ self methodClass ifNotNil: [:mc | PackageOrganizer default mostSpecificPackageOfClass: mc ifNone: []]

Looking at the senders, reveals that the method is actually not used (the most relevant user is the one we just wrote):

We remove the method. And we are almost there. There is still one thing left we could do. We could search for all messages that sent from the Settings Browser and that are implemented in PackageOrganizer but are not implemented in RPackageOrganizer:

(RPackageOrganizer default packageNamed: 'System-Settings')
  methodReferences select: [:each |
    each method parseTree allChildren anySatisfy: [ :node |
      node isMessage and: [
        (PackageOrganizer selectors includes: node selector) and: [
          (RPackageOrganizer selectors includes: node selector) not ] ] ] ] ]

This gets zero hits. But, repeating the same query for PackageInfo and RPackage gets us 24 candidate methods:

However, going a step further, we see that the problematic messages are actually not RPackage specific at all:

Now we are done.

This took about half an hour of work, and at the end of it we improved the performance of a piece of code we never saw before. We did this by systematically digging through the problem and by using appropriate analyses for performance and static checks. Along the way, code reading was used sparingly and driven by a hypothesis and custom tools.

The ability to not read code all the time is invaluable for getting productive results.