<div id="__MailbirdStyleContent" style="font-size: 10pt;font-family: Arial;color: #000000;text-align: left" dir="ltr">

                                        Hi Christoph --<div><br></div><div>I agree with Levente in that we should not change Association >> #hash.</div><div><br></div><div>+1 on your proposed changes to UserInterfaceTheme.</div><div><br></div><div>Maybe just use "UserInterfaceTheme cleanUpAndReset" as a postscript. :-) At this point, we still do not expect custom stuff in there.</div><div><br></div><div>Best,</div><div>Marcel</div><div class="mb_sig"></div>

                                        <blockquote class="history_container" type="cite" style="border-left-style: solid;border-width: 1px;margin-top: 20px;margin-left: 0px;padding-left: 10px;min-width: 500px">

                        <p style="color: #AAAAAA; margin-top: 10px;">Am 26.05.2022 23:32:27 schrieb Levente Uzonyi <leves@caesar.elte.hu>:</p><div style="font-family:Arial,Helvetica,sans-serif">Hi Christoph,

<br>

<br>On Thu, 26 May 2022, christoph.thiede@student.hpi.uni-potsdam.de wrote:

<br>

<br>> Hi all!

<br>> 

<br>> while playing around with code simulation recently, I found out that the following dictionary lookup consumes extremely many bytecodes:

<br>> 

>     UserInterfaceTheme current get: #return for: SHTextStylerST80.

<br>> 

> Dictionary >> #scanFor: takes 59 (sic!) attempts to find the right item in the dictionary. In other words, almost 20% of the property table is traversed, which does not conform my idea of a hashtable.

<br>> 

<br>> The reason for this can be found in Association >> #hash: Since Collections-ar.304, it does not take the value of an association into consideration for the hash any longer. As UserInterfaceTheme uses Associations as keys in

<br>> the Dictionary, we have a lot of "hash misses". Even though you might not notice this in practice, I noticed this indeed during debugging und simulating, I can tell you. :-)

<br>> 

<br>> On the other hand, reverting Association >> #hash to honor the value slows down Unicode class compileAll indeed (significantly: 60 ms vs. 2100 ms), as noted in Andreas' original comment:

<br>> 

<br>>     Name: Collections-ar.304

<br>>     Author: ar

<br>>     Time: 11 February 2010, 11:52:58.959 pm

<br>>     UUID: 9e97b360-adf1-f745-8f8c-562aa08d566e

<br>>     Ancestors: Collections-ar.303

<br>>     

>     Fix an age-old bug in Association>>hash which causes extreme slowdowns in compiling Unicode methods. Association>>hash does not need to hash the value; it's slow and useless.

<br>> 

<br>> So my question is: What would be the right fix for this situation? Is it

<br>> a) Do not use Associations in Dictionarys when its value matters for the lookup; or

> b) Do only use Associations in Dictionarys when its value maters for the lookup?

<br>

<br>It's the user's (the developer who decided to use a hashed collection) 

responsibility to provide keys that have a good hash distribution.

<br>If it's not possible with the default hash function, a PluggableDictionary 

should be used with a better one.

<br>

<br>In this case, however, I would use Arrays as tuples instead of 

<br>Associations because not all the keys are Associations in that dictionary, 

and Array's #hash is good enough here.

<br>Changing Association >> #hash would have too many side effects for such a 

localized problem to fix.

<br>

<br>> For a), the dictionary layout of UserInterfaceTheme needs to be changed, for which I am attaching a changeset that replaces Associations by Arrays there.* For b), the layout of litIndSet in Encoder needs to be changed, for

> instance by using Bindings instead of Associations for classPool entries. However, I'm not familiar enough with Encoder etc. to answer this question.

<br>> 

<br>> *The changeset will automatically update all UserInterfaceTheme instances. Benchmark:

<br>> 

<br>>     theme := UserInterfaceTheme current.

<br>>     random := Random seed: 20220526.

<br>>     properties := (1 to: 10000) collect: [:i | theme properties keys atRandom: random].

<br>>     [properties collect: [:ea | theme get: ea]] bench.

<br>> 

<br>> Old: 45 ms | New: 10 ms

<br>> 

<br>> Looking forward to your opinion: How should we treat Associations in Dictionarys? Of course, this is not release-critical. :-)

<br>> 

<br>> Best,

<br>> Christoph

<br>> 

<br>> =============== Postscript (accelerateUserInterfaceTheme.2.cs) ===============

<br>> 

<br>> "Postscript: Update UserInterfaceTheme instances"

<br>> 

<br>> UserInterfaceTheme allSubInstancesDo: [:theme |

<br>>     | newProperties |

<br>>     newProperties := theme properties copyEmpty.

<br>>     theme properties keysAndValuesDo: [:key :value |

<br>>         newProperties

<br>>             at: ((key isKindOf: Association)

<br>>                 ifTrue: [{key key. key value}]

<br>>                 ifFalse: [key])

<br>>             put: value].

>     theme instVarNamed: 'properties' put: newProperties].

<br>

<br>There's a method named #atomicUpdate:. I'm not sure if it's needed here, 

<br>but the name suggests that it's better to use that to update the 

<br>properties. The rest of the changes looks good to me. +1

<br>

<br>

<br>Levente<br></div></blockquote></div>