On 2022-07-09T18:55:53+00:00, commits@source.squeak.org wrote:<br>
<br>
> Christoph Thiede uploaded a new version of System to project The Trunk:<br>
> http://source.squeak.org/trunk/System-ct.1367.mcz<br>
> <br>
> ==================== Summary ====================<br>
> <br>
> Name: System-ct.1367<br>
> Author: ct<br>
> Time: 9 July 2022, 8:55:09.082532 pm<br>
> UUID: 5a63f39b-5c50-0140-b457-6c972adb1874<br>
> Ancestors: System-mt.1366<br>
> <br>
> Merges accelerateUserInterfaceTheme:<br>
> Accelerates property accesses to UserInterfaceTheme. This is done by changing the dictionary layout and representing keys as Arrays instead of Associations to improve the hash distribution. The postscript will automatically convert all UserInterfaceTheem subinstances.<br>
> <br>
> Revision:<br>
> Use #atomicUpdate: in postscript.<br>
> <br>
> This speeds up theme requests by >3750% and even speeds up common toolbuilding workloads by ~48% (see benchmarks below).<br>
<br>
On slower VMs, the speed-ups are even larger!<br>
<br>
SqueakJS: Theme requests speeded up by >6800% and toolbuilder speeded up by ~58%. :-)<br>
<br>
> <br>
> Thanks to Levente (ul) and Marcel (mt) for the review! For more information, see: http://lists.squeakfoundation.org/pipermail/squeak-dev/2022-May/220694.html<br>
> <br>
> ---<br>
> <br>
> Detailed benchmarks and results from my machine:<br>
> <br>
> I. Theme requests<br>
> <br>
> | theme random keys properties |<br>
> theme := UserInterfaceTheme current.<br>
> random := Random seed: 20220526.<br>
> keys := theme properties keys sorted: [:key | ((key isKindOf: Association) ifTrue: [{key key. key value}] ifFalse: [key]) hash] ascending.<br>
> properties := (1 to: 1000000) collect: [:i | keys atRandom: random].<br>
> [properties do: [:ea | theme get: ea]] timeToRun.<br>
> "Average of 8 runs:<br>
> BEFORE: 3133.0 ms<br>
> AFTER: 655.25 ms"<br>
> <br>
> II. Toolbuilding workloads<br>
> <br>
> [ToolBuilder build: Browser] benchFor: 30 seconds.<br>
> "Average of 3 runs:<br>
> BEFORE: 19.4 ms<br>
> AFTER: 13.1 ms"<br>
> <br>
> =============== Diff against System-mt.1366 ===============<br>
> <br>
> Item was changed:<br>
> ----- Method: UserInterfaceTheme>>get: (in category 'private') -----<br>
> get: keyObject <br>
> "keyObject is intended to be an Association. We have two lookup strategies: 1) along the superclass chain *of the client*, 2) via a linked theme. Evaluate the result because there can be message sends stored or blocks."<br>
> <br>
> | k |<br>
> properties<br>
> at: keyObject<br>
> ifPresent: [:prop | ^ prop value].<br>
> <br>
> + keyObject isArray "simple key objects"<br>
> - keyObject isVariableBinding "simple key objects"<br>
> ifFalse: [^ self getViaLink: keyObject].<br>
> <br>
> + k := keyObject at: 1.<br>
> - k := keyObject key.<br>
> (self getViaSuperclasses: keyObject)<br>
> ifNotNil: [:prop | ^ prop].<br>
> <br>
> + keyObject at: 1 put: k. "restore"<br>
> - keyObject key: k. "restore"<br>
> ^ self getViaLink: keyObject!<br>
> <br>
> Item was changed:<br>
> ----- Method: UserInterfaceTheme>>get:for: (in category 'private') -----<br>
> get: propertySymbol for: scope<br>
> "For convenience. Does support access to non-class keys."<br>
> <br>
> | aClass |<br>
> aClass := (scope isNil or: [scope isBehavior])<br>
> ifTrue: [scope]<br>
> ifFalse: [Smalltalk classNamed: scope].<br>
> <br>
> + aClass ifNotNil: [^ self get: {aClass. propertySymbol}].<br>
> - aClass ifNotNil: [^ self get: aClass -> propertySymbol].<br>
> <br>
> properties<br>
> + at: {scope. propertySymbol}<br>
> - at: scope -> propertySymbol<br>
> ifPresent: [:prop | ^ prop value].<br>
> <br>
> + ^ self getViaLink: {scope. propertySymbol}!<br>
> - ^ self getViaLink: scope -> propertySymbol!<br>
> <br>
> Item was changed:<br>
> ----- Method: UserInterfaceTheme>>getViaSuperclasses: (in category 'private') -----<br>
> getViaSuperclasses: keyObject <br>
> "keyObject is intended to be an Association.<br>
> Find the superclass of the key of the keyObject (which will initially be the client's class) and make a new keyObject using that and the original message name, then try searching for that."<br>
> <br>
> "We know we're the only referencer of keyObject. Update it rather than create new ones, for performance reasons."<br>
> + keyObject at: 1 put: (keyObject at: 1) superclass.<br>
> - keyObject key: keyObject key superclass.<br>
> <br>
> + (keyObject at: 1) ifNil: [^ nil].<br>
> - keyObject key ifNil: [^ nil].<br>
> <br>
> properties<br>
> at: keyObject<br>
> ifPresent: [:prop | ^ prop value].<br>
> <br>
> ^ self getViaSuperclasses: keyObject!<br>
> <br>
> Item was changed:<br>
> ----- Method: UserInterfaceTheme>>set:for:to: (in category 'building') -----<br>
> set: propertySymbol for: aClassOrSymbol to: valueObject<br>
> "Where aClass asks its userInterfaceTheme for propertySymbol, provide valueObject."<br>
> <br>
> | aClass |<br>
> aClass := aClassOrSymbol isBehavior ifTrue: [aClassOrSymbol] ifFalse: [Smalltalk classNamed: aClassOrSymbol].<br>
> aClass ifNil: [^ self].<br>
> ^ self atomicUpdate:<br>
> [ : props | | key |<br>
> + key := {aClass. propertySymbol}.<br>
> - key := aClass -> propertySymbol.<br>
> valueObject<br>
> ifNil:<br>
> [ props<br>
> removeKey: key<br>
> ifAbsent: [ "already cleared, don't error" ] ]<br>
> ifNotNil:<br>
> [ props<br>
> at: key<br>
> put: valueObject ] ]!<br>
> <br>
> Item was changed:<br>
> ----- Method: UserInterfaceThemeRequest>>doesNotUnderstand: (in category 'lookup') -----<br>
> doesNotUnderstand: aMessage <br>
> "Look up the visual attribute specified by aMessage's #selector in the current theme for the current target object."<br>
> <br>
> aMessage numArgs = 0 ifTrue: [<br>
> + ^ (self theme get: {self target class. aMessage selector})<br>
> - ^ (self theme get: self target class -> aMessage selector)<br>
> ifNil: [(self theme respondsTo: aMessage selector)<br>
> ifTrue: [self theme perform: aMessage selector]<br>
> ifFalse: [nil "unset property"]]].<br>
> <br>
> aMessage numArgs = 1 ifTrue: [<br>
> ^ self theme<br>
> + set: {self target class. aMessage selector asSimpleGetter}<br>
> - set: self target class -> aMessage selector asSimpleGetter<br>
> to: aMessage arguments first].<br>
> <br>
> ^ self theme<br>
> perform: aMessage selector<br>
> withArguments: aMessage arguments.!<br>
> <br>
> Item was changed:<br>
> + (PackageInfo named: 'System') postscript: '"Update UserInterfaceTheme instances (System-ct.1367)"<br>
> + UserInterfaceTheme allSubInstancesDo: [:theme |<br>
> + theme atomicUpdate: [:props |<br>
> + props copy keysAndValuesDo: [:key :value |<br>
> + (key isKindOf: Association) ifTrue: [<br>
> + props<br>
> + removeKey: key;<br>
> + at: {key key. key value} put: value]]]].'!<br>
> - (PackageInfo named: 'System') postscript: 'SystemNavigation initializeAuthors.. "Adds Eric Gade"'!<br>
> <br>
<br>
Best,<br>
Christoph<br>
<br>
<font color="#808080">---<br>
</font><font color="#808080"><i>Sent from </i></font><font color="#808080"><i><a href="https://github.com/hpi-swa-lab/squeak-inbox-talk"><u><font color="#808080">Squeak Inbox Talk</font></u></a></i></font>