Hi,
We have a server application that, after some longer usage, becomes very slow. It turned out that this is because lots of symbols are created from strings (without holding onto them), which becomes slower and slower over time. This is easy to fix in our app, but I'd like to understand why Squeak has a problem with that. Using the following snipped, I produced the attached chart (Y axis is ms spent to create 5000 random symbols). You can nicely see the reoccurring pattern that grows over time. 1000 timesRepeat: [ Transcript print: ([ 5000 timesRepeat: [ (($a to: $x) atRandom asString , 'asdasd') asSymbol ] ] timeToRun); cr; flush. ] Message tally sais: 96.2% {13872ms} ByteString(String)>>asSymbol |96.2% {13872ms} Symbol class>>intern: | 49.7% {7167ms} Symbol class>>lookup: | |49.7% {7167ms} WeakSet>>like: | | 49.7% {7167ms} WeakSet>>scanFor: | | 32.3% {4658ms} primitives | | 17.3% {2495ms} UndefinedObject(Object)>>= | 44.4% {6402ms} WeakSet>>add: | |44.3% {6388ms} WeakSet(Set)>>findElementOrNil: | | 44.3% {6388ms} WeakSet>>scanFor: | | 22.8% {3288ms} UndefinedObject(Object)>>= | | 21.5% {3100ms} primitives | 2.1% {303ms} ByteSymbol>>string: 2.9% {418ms} Character>>to: 2.4% {346ms} SmallInteger(Number)>>to: 2.4% {346ms} Interval class>>from:to:by: 2.4% {346ms} Interval>>setFrom:to:by: I haven't had time to look into this very closely yet but maybe somebody else already knows what is going on. My guess is that the NewSymbols weak set is repeatedly grown since tally is not decreased if the GC removes a weak element. This is a Squeak 3.9 image (OS X and Linux). Cheers, Adrian ___________________ http://www.adrian-lienhard.ch/ Picture 2.png (18K) Download Attachment |
Adrian Lienhard wrote:
> I haven't had time to look into this very closely yet but maybe somebody > else already knows what is going on. My guess is that the NewSymbols > weak set is repeatedly grown since tally is not decreased if the GC > removes a weak element. That's an excellent guess. Try changing WeakSet to e.g., WeakSet>>atNewIndex: index put: aValue "Add newValue at the given index. Don't increment tally if current slot is nil since a previous value has been collected" (array at: index) ifNil:[^array at: index put: aValue]. ^super atNewIndex: index put: aValue Cheers, - Andreas |
Isn't that dangerous if you don't fixCollisionsFrom: index ?
Nicolas 2009/5/6 Andreas Raab <[hidden email]>: > Adrian Lienhard wrote: >> >> I haven't had time to look into this very closely yet but maybe somebody >> else already knows what is going on. My guess is that the NewSymbols weak >> set is repeatedly grown since tally is not decreased if the GC removes a >> weak element. > > That's an excellent guess. Try changing WeakSet to e.g., > > WeakSet>>atNewIndex: index put: aValue > "Add newValue at the given index. Don't increment tally > if current slot is nil since a previous value has been collected" > (array at: index) ifNil:[^array at: index put: aValue]. > ^super atNewIndex: index put: aValue > > Cheers, > - Andreas > > > |
Nicolas Cellier wrote:
> Isn't that dangerous if you don't fixCollisionsFrom: index ? Why would it? All this does is avoiding to update the tally ivar if we're replacing a previously reclaimed entry with a new value. And I don't think that #fixCollisionsFrom: even looks at tally. Cheers, - Andreas > Nicolas > > 2009/5/6 Andreas Raab <[hidden email]>: >> Adrian Lienhard wrote: >>> I haven't had time to look into this very closely yet but maybe somebody >>> else already knows what is going on. My guess is that the NewSymbols weak >>> set is repeatedly grown since tally is not decreased if the GC removes a >>> weak element. >> That's an excellent guess. Try changing WeakSet to e.g., >> >> WeakSet>>atNewIndex: index put: aValue >> "Add newValue at the given index. Don't increment tally >> if current slot is nil since a previous value has been collected" >> (array at: index) ifNil:[^array at: index put: aValue]. >> ^super atNewIndex: index put: aValue >> >> Cheers, >> - Andreas >> >> >> > > |
Take this example:
ws := WeakSet new. array := WeakSet new instVarAt: (WeakSet allInstVarNames indexOf: 'array'). o1 hash \\ array size + 1 -> 1. o2 hash \\ array size + 1 -> 2. o3 hash \\ array size + 1 -> 1. ws add: o1; add: o2; add: o3. o1 := o2 := nil. Smalltalk garbageCollect. "o1 and o2 are reclaimed." (array at: 1) -> nil. (array at: 2) -> nil. o4 hash \\ array size + 1 -> 1. ws add o4. (array at: 1) -> o4. (ws includes: o3) -> false. I will try and build a test case. Should be easy with Fractions. 2009/5/6 Andreas Raab <[hidden email]>: > Nicolas Cellier wrote: >> >> Isn't that dangerous if you don't fixCollisionsFrom: index ? > > Why would it? All this does is avoiding to update the tally ivar if we're > replacing a previously reclaimed entry with a new value. And I don't think > that #fixCollisionsFrom: even looks at tally. > > Cheers, > - Andreas > >> Nicolas >> >> 2009/5/6 Andreas Raab <[hidden email]>: >>> >>> Adrian Lienhard wrote: >>>> >>>> I haven't had time to look into this very closely yet but maybe somebody >>>> else already knows what is going on. My guess is that the NewSymbols >>>> weak >>>> set is repeatedly grown since tally is not decreased if the GC removes a >>>> weak element. >>> >>> That's an excellent guess. Try changing WeakSet to e.g., >>> >>> WeakSet>>atNewIndex: index put: aValue >>> "Add newValue at the given index. Don't increment tally >>> if current slot is nil since a previous value has been collected" >>> (array at: index) ifNil:[^array at: index put: aValue]. >>> ^super atNewIndex: index put: aValue >>> >>> Cheers, >>> - Andreas >>> >>> >>> >> >> > > > |
Nicolas Cellier wrote:
> Take this example: Ah, good point. (it works fine with o1 = 'g' and o2 = o3 = o4 = 'd'). The thing is that the test in WeakSet>>add: was misleading - it tests for nil where scanFor: searches only for flag. Cheers, - Andreas > ws := WeakSet new. > array := WeakSet new instVarAt: (WeakSet allInstVarNames indexOf: 'array'). > o1 hash \\ array size + 1 -> 1. > o2 hash \\ array size + 1 -> 2. > o3 hash \\ array size + 1 -> 1. > ws add: o1; add: o2; add: o3. > > o1 := o2 := nil. > Smalltalk garbageCollect. > "o1 and o2 are reclaimed." > (array at: 1) -> nil. > (array at: 2) -> nil. > > o4 hash \\ array size + 1 -> 1. > ws add o4. > (array at: 1) -> o4. > > (ws includes: o3) -> false. > > I will try and build a test case. Should be easy with Fractions. > > 2009/5/6 Andreas Raab <[hidden email]>: >> Nicolas Cellier wrote: >>> Isn't that dangerous if you don't fixCollisionsFrom: index ? >> Why would it? All this does is avoiding to update the tally ivar if we're >> replacing a previously reclaimed entry with a new value. And I don't think >> that #fixCollisionsFrom: even looks at tally. >> >> Cheers, >> - Andreas >> >>> Nicolas >>> >>> 2009/5/6 Andreas Raab <[hidden email]>: >>>> Adrian Lienhard wrote: >>>>> I haven't had time to look into this very closely yet but maybe somebody >>>>> else already knows what is going on. My guess is that the NewSymbols >>>>> weak >>>>> set is repeatedly grown since tally is not decreased if the GC removes a >>>>> weak element. >>>> That's an excellent guess. Try changing WeakSet to e.g., >>>> >>>> WeakSet>>atNewIndex: index put: aValue >>>> "Add newValue at the given index. Don't increment tally >>>> if current slot is nil since a previous value has been collected" >>>> (array at: index) ifNil:[^array at: index put: aValue]. >>>> ^super atNewIndex: index put: aValue >>>> >>>> Cheers, >>>> - Andreas >>>> >>>> >>>> >>> >> >> > > |
Though I have tried to attack it with some tricky combinations, I did
not found a problem with current implementation. It was funny to see how nil is handled as any other object in WeakSet fixCollisionsFrom: (scanFor: nil works). However, it's a pity not to recycle all those nils until next grow! Here comes a little trial for recycling reclaimed nil slots in Pharo (seems compatible with 3.10.2). No guarantee... Note there is no WeakSet tests in Pharo nor 3.10.2 currently :( Nicolas 2009/5/6 Andreas Raab <[hidden email]>: > Nicolas Cellier wrote: >> >> Take this example: > > Ah, good point. (it works fine with o1 = 'g' and o2 = o3 = o4 = 'd'). The > thing is that the test in WeakSet>>add: was misleading - it tests for nil > where scanFor: searches only for flag. > > Cheers, > - Andreas > >> ws := WeakSet new. >> array := WeakSet new instVarAt: (WeakSet allInstVarNames indexOf: >> 'array'). >> o1 hash \\ array size + 1 -> 1. >> o2 hash \\ array size + 1 -> 2. >> o3 hash \\ array size + 1 -> 1. >> ws add: o1; add: o2; add: o3. >> >> o1 := o2 := nil. >> Smalltalk garbageCollect. >> "o1 and o2 are reclaimed." >> (array at: 1) -> nil. >> (array at: 2) -> nil. >> >> o4 hash \\ array size + 1 -> 1. >> ws add o4. >> (array at: 1) -> o4. >> >> (ws includes: o3) -> false. >> >> I will try and build a test case. Should be easy with Fractions. >> >> 2009/5/6 Andreas Raab <[hidden email]>: >>> >>> Nicolas Cellier wrote: >>>> >>>> Isn't that dangerous if you don't fixCollisionsFrom: index ? >>> >>> Why would it? All this does is avoiding to update the tally ivar if we're >>> replacing a previously reclaimed entry with a new value. And I don't >>> think >>> that #fixCollisionsFrom: even looks at tally. >>> >>> Cheers, >>> - Andreas >>> >>>> Nicolas >>>> >>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>> >>>>> Adrian Lienhard wrote: >>>>>> >>>>>> I haven't had time to look into this very closely yet but maybe >>>>>> somebody >>>>>> else already knows what is going on. My guess is that the NewSymbols >>>>>> weak >>>>>> set is repeatedly grown since tally is not decreased if the GC removes >>>>>> a >>>>>> weak element. >>>>> >>>>> That's an excellent guess. Try changing WeakSet to e.g., >>>>> >>>>> WeakSet>>atNewIndex: index put: aValue >>>>> "Add newValue at the given index. Don't increment tally >>>>> if current slot is nil since a previous value has been collected" >>>>> (array at: index) ifNil:[^array at: index put: aValue]. >>>>> ^super atNewIndex: index put: aValue >>>>> >>>>> Cheers, >>>>> - Andreas >>>>> >>>>> >>>>> >>>> >>> >>> >> >> > > > WeakSet-recycleNilSlots-nice.1.cs (2K) Download Attachment |
Ah, I forgot to accept one change!
It's even better if we decrease the tally when recycling a nil slot in #fixCollisionsFrom: Nicolas 2009/5/6 Nicolas Cellier <[hidden email]>: > Though I have tried to attack it with some tricky combinations, I did > not found a problem with current implementation. > It was funny to see how nil is handled as any other object in WeakSet > fixCollisionsFrom: (scanFor: nil works). > > However, it's a pity not to recycle all those nils until next grow! > Here comes a little trial for recycling reclaimed nil slots in Pharo > (seems compatible with 3.10.2). > No guarantee... Note there is no WeakSet tests in Pharo nor 3.10.2 currently :( > > Nicolas > > 2009/5/6 Andreas Raab <[hidden email]>: >> Nicolas Cellier wrote: >>> >>> Take this example: >> >> Ah, good point. (it works fine with o1 = 'g' and o2 = o3 = o4 = 'd'). The >> thing is that the test in WeakSet>>add: was misleading - it tests for nil >> where scanFor: searches only for flag. >> >> Cheers, >> - Andreas >> >>> ws := WeakSet new. >>> array := WeakSet new instVarAt: (WeakSet allInstVarNames indexOf: >>> 'array'). >>> o1 hash \\ array size + 1 -> 1. >>> o2 hash \\ array size + 1 -> 2. >>> o3 hash \\ array size + 1 -> 1. >>> ws add: o1; add: o2; add: o3. >>> >>> o1 := o2 := nil. >>> Smalltalk garbageCollect. >>> "o1 and o2 are reclaimed." >>> (array at: 1) -> nil. >>> (array at: 2) -> nil. >>> >>> o4 hash \\ array size + 1 -> 1. >>> ws add o4. >>> (array at: 1) -> o4. >>> >>> (ws includes: o3) -> false. >>> >>> I will try and build a test case. Should be easy with Fractions. >>> >>> 2009/5/6 Andreas Raab <[hidden email]>: >>>> >>>> Nicolas Cellier wrote: >>>>> >>>>> Isn't that dangerous if you don't fixCollisionsFrom: index ? >>>> >>>> Why would it? All this does is avoiding to update the tally ivar if we're >>>> replacing a previously reclaimed entry with a new value. And I don't >>>> think >>>> that #fixCollisionsFrom: even looks at tally. >>>> >>>> Cheers, >>>> - Andreas >>>> >>>>> Nicolas >>>>> >>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>> >>>>>> Adrian Lienhard wrote: >>>>>>> >>>>>>> I haven't had time to look into this very closely yet but maybe >>>>>>> somebody >>>>>>> else already knows what is going on. My guess is that the NewSymbols >>>>>>> weak >>>>>>> set is repeatedly grown since tally is not decreased if the GC removes >>>>>>> a >>>>>>> weak element. >>>>>> >>>>>> That's an excellent guess. Try changing WeakSet to e.g., >>>>>> >>>>>> WeakSet>>atNewIndex: index put: aValue >>>>>> "Add newValue at the given index. Don't increment tally >>>>>> if current slot is nil since a previous value has been collected" >>>>>> (array at: index) ifNil:[^array at: index put: aValue]. >>>>>> ^super atNewIndex: index put: aValue >>>>>> >>>>>> Cheers, >>>>>> - Andreas >>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>> >>> >> >> >> > WeakSet-recycleNilSlots-nice.2.cs (2K) Download Attachment |
Created http://bugs.squeak.org/view.php?id=7350
2009/5/6 Nicolas Cellier <[hidden email]>: > Ah, I forgot to accept one change! > It's even better if we decrease the tally when recycling a nil slot in > #fixCollisionsFrom: > > Nicolas > > 2009/5/6 Nicolas Cellier <[hidden email]>: >> Though I have tried to attack it with some tricky combinations, I did >> not found a problem with current implementation. >> It was funny to see how nil is handled as any other object in WeakSet >> fixCollisionsFrom: (scanFor: nil works). >> >> However, it's a pity not to recycle all those nils until next grow! >> Here comes a little trial for recycling reclaimed nil slots in Pharo >> (seems compatible with 3.10.2). >> No guarantee... Note there is no WeakSet tests in Pharo nor 3.10.2 currently :( >> >> Nicolas >> >> 2009/5/6 Andreas Raab <[hidden email]>: >>> Nicolas Cellier wrote: >>>> >>>> Take this example: >>> >>> Ah, good point. (it works fine with o1 = 'g' and o2 = o3 = o4 = 'd'). The >>> thing is that the test in WeakSet>>add: was misleading - it tests for nil >>> where scanFor: searches only for flag. >>> >>> Cheers, >>> - Andreas >>> >>>> ws := WeakSet new. >>>> array := WeakSet new instVarAt: (WeakSet allInstVarNames indexOf: >>>> 'array'). >>>> o1 hash \\ array size + 1 -> 1. >>>> o2 hash \\ array size + 1 -> 2. >>>> o3 hash \\ array size + 1 -> 1. >>>> ws add: o1; add: o2; add: o3. >>>> >>>> o1 := o2 := nil. >>>> Smalltalk garbageCollect. >>>> "o1 and o2 are reclaimed." >>>> (array at: 1) -> nil. >>>> (array at: 2) -> nil. >>>> >>>> o4 hash \\ array size + 1 -> 1. >>>> ws add o4. >>>> (array at: 1) -> o4. >>>> >>>> (ws includes: o3) -> false. >>>> >>>> I will try and build a test case. Should be easy with Fractions. >>>> >>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>> >>>>> Nicolas Cellier wrote: >>>>>> >>>>>> Isn't that dangerous if you don't fixCollisionsFrom: index ? >>>>> >>>>> Why would it? All this does is avoiding to update the tally ivar if we're >>>>> replacing a previously reclaimed entry with a new value. And I don't >>>>> think >>>>> that #fixCollisionsFrom: even looks at tally. >>>>> >>>>> Cheers, >>>>> - Andreas >>>>> >>>>>> Nicolas >>>>>> >>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>> >>>>>>> Adrian Lienhard wrote: >>>>>>>> >>>>>>>> I haven't had time to look into this very closely yet but maybe >>>>>>>> somebody >>>>>>>> else already knows what is going on. My guess is that the NewSymbols >>>>>>>> weak >>>>>>>> set is repeatedly grown since tally is not decreased if the GC removes >>>>>>>> a >>>>>>>> weak element. >>>>>>> >>>>>>> That's an excellent guess. Try changing WeakSet to e.g., >>>>>>> >>>>>>> WeakSet>>atNewIndex: index put: aValue >>>>>>> "Add newValue at the given index. Don't increment tally >>>>>>> if current slot is nil since a previous value has been collected" >>>>>>> (array at: index) ifNil:[^array at: index put: aValue]. >>>>>>> ^super atNewIndex: index put: aValue >>>>>>> >>>>>>> Cheers, >>>>>>> - Andreas >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>> >>> >>> >> > |
Nicolas Cellier wrote:
> Created http://bugs.squeak.org/view.php?id=7350 Thanks, this is great. Is this approach applicable to weak dictionaries as well? Cheers, - Andreas > 2009/5/6 Nicolas Cellier <[hidden email]>: >> Ah, I forgot to accept one change! >> It's even better if we decrease the tally when recycling a nil slot in >> #fixCollisionsFrom: >> >> Nicolas >> >> 2009/5/6 Nicolas Cellier <[hidden email]>: >>> Though I have tried to attack it with some tricky combinations, I did >>> not found a problem with current implementation. >>> It was funny to see how nil is handled as any other object in WeakSet >>> fixCollisionsFrom: (scanFor: nil works). >>> >>> However, it's a pity not to recycle all those nils until next grow! >>> Here comes a little trial for recycling reclaimed nil slots in Pharo >>> (seems compatible with 3.10.2). >>> No guarantee... Note there is no WeakSet tests in Pharo nor 3.10.2 currently :( >>> >>> Nicolas >>> >>> 2009/5/6 Andreas Raab <[hidden email]>: >>>> Nicolas Cellier wrote: >>>>> Take this example: >>>> Ah, good point. (it works fine with o1 = 'g' and o2 = o3 = o4 = 'd'). The >>>> thing is that the test in WeakSet>>add: was misleading - it tests for nil >>>> where scanFor: searches only for flag. >>>> >>>> Cheers, >>>> - Andreas >>>> >>>>> ws := WeakSet new. >>>>> array := WeakSet new instVarAt: (WeakSet allInstVarNames indexOf: >>>>> 'array'). >>>>> o1 hash \\ array size + 1 -> 1. >>>>> o2 hash \\ array size + 1 -> 2. >>>>> o3 hash \\ array size + 1 -> 1. >>>>> ws add: o1; add: o2; add: o3. >>>>> >>>>> o1 := o2 := nil. >>>>> Smalltalk garbageCollect. >>>>> "o1 and o2 are reclaimed." >>>>> (array at: 1) -> nil. >>>>> (array at: 2) -> nil. >>>>> >>>>> o4 hash \\ array size + 1 -> 1. >>>>> ws add o4. >>>>> (array at: 1) -> o4. >>>>> >>>>> (ws includes: o3) -> false. >>>>> >>>>> I will try and build a test case. Should be easy with Fractions. >>>>> >>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>> Nicolas Cellier wrote: >>>>>>> Isn't that dangerous if you don't fixCollisionsFrom: index ? >>>>>> Why would it? All this does is avoiding to update the tally ivar if we're >>>>>> replacing a previously reclaimed entry with a new value. And I don't >>>>>> think >>>>>> that #fixCollisionsFrom: even looks at tally. >>>>>> >>>>>> Cheers, >>>>>> - Andreas >>>>>> >>>>>>> Nicolas >>>>>>> >>>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>>> Adrian Lienhard wrote: >>>>>>>>> I haven't had time to look into this very closely yet but maybe >>>>>>>>> somebody >>>>>>>>> else already knows what is going on. My guess is that the NewSymbols >>>>>>>>> weak >>>>>>>>> set is repeatedly grown since tally is not decreased if the GC removes >>>>>>>>> a >>>>>>>>> weak element. >>>>>>>> That's an excellent guess. Try changing WeakSet to e.g., >>>>>>>> >>>>>>>> WeakSet>>atNewIndex: index put: aValue >>>>>>>> "Add newValue at the given index. Don't increment tally >>>>>>>> if current slot is nil since a previous value has been collected" >>>>>>>> (array at: index) ifNil:[^array at: index put: aValue]. >>>>>>>> ^super atNewIndex: index put: aValue >>>>>>>> >>>>>>>> Cheers, >>>>>>>> - Andreas >>>>>>>> >>>>>>>> >>>>>>>> >>>>>> >>>>> >>>> >>>> > > |
Wait, there is a bug in scanFor: fixCollisionsFrom: does not recycle
the first nil and scanFor: then loops indefinitely :( see http://bugs.squeak.org/view.php?id=7350 for update 2009/5/7 Andreas Raab <[hidden email]>: > Nicolas Cellier wrote: >> >> Created http://bugs.squeak.org/view.php?id=7350 > > Thanks, this is great. Is this approach applicable to weak dictionaries as > well? > Should inquire, but it's getting late... > Cheers, > - Andreas > >> 2009/5/6 Nicolas Cellier <[hidden email]>: >>> >>> Ah, I forgot to accept one change! >>> It's even better if we decrease the tally when recycling a nil slot in >>> #fixCollisionsFrom: >>> >>> Nicolas >>> >>> 2009/5/6 Nicolas Cellier <[hidden email]>: >>>> >>>> Though I have tried to attack it with some tricky combinations, I did >>>> not found a problem with current implementation. >>>> It was funny to see how nil is handled as any other object in WeakSet >>>> fixCollisionsFrom: (scanFor: nil works). >>>> >>>> However, it's a pity not to recycle all those nils until next grow! >>>> Here comes a little trial for recycling reclaimed nil slots in Pharo >>>> (seems compatible with 3.10.2). >>>> No guarantee... Note there is no WeakSet tests in Pharo nor 3.10.2 >>>> currently :( >>>> >>>> Nicolas >>>> >>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>> >>>>> Nicolas Cellier wrote: >>>>>> >>>>>> Take this example: >>>>> >>>>> Ah, good point. (it works fine with o1 = 'g' and o2 = o3 = o4 = 'd'). >>>>> The >>>>> thing is that the test in WeakSet>>add: was misleading - it tests for >>>>> nil >>>>> where scanFor: searches only for flag. >>>>> >>>>> Cheers, >>>>> - Andreas >>>>> >>>>>> ws := WeakSet new. >>>>>> array := WeakSet new instVarAt: (WeakSet allInstVarNames indexOf: >>>>>> 'array'). >>>>>> o1 hash \\ array size + 1 -> 1. >>>>>> o2 hash \\ array size + 1 -> 2. >>>>>> o3 hash \\ array size + 1 -> 1. >>>>>> ws add: o1; add: o2; add: o3. >>>>>> >>>>>> o1 := o2 := nil. >>>>>> Smalltalk garbageCollect. >>>>>> "o1 and o2 are reclaimed." >>>>>> (array at: 1) -> nil. >>>>>> (array at: 2) -> nil. >>>>>> >>>>>> o4 hash \\ array size + 1 -> 1. >>>>>> ws add o4. >>>>>> (array at: 1) -> o4. >>>>>> >>>>>> (ws includes: o3) -> false. >>>>>> >>>>>> I will try and build a test case. Should be easy with Fractions. >>>>>> >>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>> >>>>>>> Nicolas Cellier wrote: >>>>>>>> >>>>>>>> Isn't that dangerous if you don't fixCollisionsFrom: index ? >>>>>>> >>>>>>> Why would it? All this does is avoiding to update the tally ivar if >>>>>>> we're >>>>>>> replacing a previously reclaimed entry with a new value. And I don't >>>>>>> think >>>>>>> that #fixCollisionsFrom: even looks at tally. >>>>>>> >>>>>>> Cheers, >>>>>>> - Andreas >>>>>>> >>>>>>>> Nicolas >>>>>>>> >>>>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>>>> >>>>>>>>> Adrian Lienhard wrote: >>>>>>>>>> >>>>>>>>>> I haven't had time to look into this very closely yet but maybe >>>>>>>>>> somebody >>>>>>>>>> else already knows what is going on. My guess is that the >>>>>>>>>> NewSymbols >>>>>>>>>> weak >>>>>>>>>> set is repeatedly grown since tally is not decreased if the GC >>>>>>>>>> removes >>>>>>>>>> a >>>>>>>>>> weak element. >>>>>>>>> >>>>>>>>> That's an excellent guess. Try changing WeakSet to e.g., >>>>>>>>> >>>>>>>>> WeakSet>>atNewIndex: index put: aValue >>>>>>>>> "Add newValue at the given index. Don't increment tally >>>>>>>>> if current slot is nil since a previous value has been collected" >>>>>>>>> (array at: index) ifNil:[^array at: index put: aValue]. >>>>>>>>> ^super atNewIndex: index put: aValue >>>>>>>>> >>>>>>>>> Cheers, >>>>>>>>> - Andreas >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >> >> > > > |
2009/5/7 Nicolas Cellier <[hidden email]>:
> Wait, there is a bug in scanFor: fixCollisionsFrom: does not recycle > the first nil and scanFor: then loops indefinitely :( see > http://bugs.squeak.org/view.php?id=7350 for update > > 2009/5/7 Andreas Raab <[hidden email]>: >> Nicolas Cellier wrote: >>> >>> Created http://bugs.squeak.org/view.php?id=7350 >> >> Thanks, this is great. Is this approach applicable to weak dictionaries as >> well? >> > > Should inquire, but it's getting late... Oh yes, it could... but WeakRegistry will not appreciate those assoc values being garbaged out without calling #finalize first... Maybe #finalize could be sent directly by WeakKeyDictionary ? WeakRegistry are protected forMutualExclusion, WeakKeyDictionary is not. So WeakRegistry must stay. There might be more subtle things you know better than me... ... like finalizeValues is two folds: 1) clean-up valueDictionary in a protected block 2) finalize outside this block Didn't get why... could #finalize have a chance to modify valueDictionary ? Nicolas > >> Cheers, >> - Andreas >> >>> 2009/5/6 Nicolas Cellier <[hidden email]>: >>>> >>>> Ah, I forgot to accept one change! >>>> It's even better if we decrease the tally when recycling a nil slot in >>>> #fixCollisionsFrom: >>>> >>>> Nicolas >>>> >>>> 2009/5/6 Nicolas Cellier <[hidden email]>: >>>>> >>>>> Though I have tried to attack it with some tricky combinations, I did >>>>> not found a problem with current implementation. >>>>> It was funny to see how nil is handled as any other object in WeakSet >>>>> fixCollisionsFrom: (scanFor: nil works). >>>>> >>>>> However, it's a pity not to recycle all those nils until next grow! >>>>> Here comes a little trial for recycling reclaimed nil slots in Pharo >>>>> (seems compatible with 3.10.2). >>>>> No guarantee... Note there is no WeakSet tests in Pharo nor 3.10.2 >>>>> currently :( >>>>> >>>>> Nicolas >>>>> >>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>> >>>>>> Nicolas Cellier wrote: >>>>>>> >>>>>>> Take this example: >>>>>> >>>>>> Ah, good point. (it works fine with o1 = 'g' and o2 = o3 = o4 = 'd'). >>>>>> The >>>>>> thing is that the test in WeakSet>>add: was misleading - it tests for >>>>>> nil >>>>>> where scanFor: searches only for flag. >>>>>> >>>>>> Cheers, >>>>>> - Andreas >>>>>> >>>>>>> ws := WeakSet new. >>>>>>> array := WeakSet new instVarAt: (WeakSet allInstVarNames indexOf: >>>>>>> 'array'). >>>>>>> o1 hash \\ array size + 1 -> 1. >>>>>>> o2 hash \\ array size + 1 -> 2. >>>>>>> o3 hash \\ array size + 1 -> 1. >>>>>>> ws add: o1; add: o2; add: o3. >>>>>>> >>>>>>> o1 := o2 := nil. >>>>>>> Smalltalk garbageCollect. >>>>>>> "o1 and o2 are reclaimed." >>>>>>> (array at: 1) -> nil. >>>>>>> (array at: 2) -> nil. >>>>>>> >>>>>>> o4 hash \\ array size + 1 -> 1. >>>>>>> ws add o4. >>>>>>> (array at: 1) -> o4. >>>>>>> >>>>>>> (ws includes: o3) -> false. >>>>>>> >>>>>>> I will try and build a test case. Should be easy with Fractions. >>>>>>> >>>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>>> >>>>>>>> Nicolas Cellier wrote: >>>>>>>>> >>>>>>>>> Isn't that dangerous if you don't fixCollisionsFrom: index ? >>>>>>>> >>>>>>>> Why would it? All this does is avoiding to update the tally ivar if >>>>>>>> we're >>>>>>>> replacing a previously reclaimed entry with a new value. And I don't >>>>>>>> think >>>>>>>> that #fixCollisionsFrom: even looks at tally. >>>>>>>> >>>>>>>> Cheers, >>>>>>>> - Andreas >>>>>>>> >>>>>>>>> Nicolas >>>>>>>>> >>>>>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>>>>> >>>>>>>>>> Adrian Lienhard wrote: >>>>>>>>>>> >>>>>>>>>>> I haven't had time to look into this very closely yet but maybe >>>>>>>>>>> somebody >>>>>>>>>>> else already knows what is going on. My guess is that the >>>>>>>>>>> NewSymbols >>>>>>>>>>> weak >>>>>>>>>>> set is repeatedly grown since tally is not decreased if the GC >>>>>>>>>>> removes >>>>>>>>>>> a >>>>>>>>>>> weak element. >>>>>>>>>> >>>>>>>>>> That's an excellent guess. Try changing WeakSet to e.g., >>>>>>>>>> >>>>>>>>>> WeakSet>>atNewIndex: index put: aValue >>>>>>>>>> "Add newValue at the given index. Don't increment tally >>>>>>>>>> if current slot is nil since a previous value has been collected" >>>>>>>>>> (array at: index) ifNil:[^array at: index put: aValue]. >>>>>>>>>> ^super atNewIndex: index put: aValue >>>>>>>>>> >>>>>>>>>> Cheers, >>>>>>>>>> - Andreas >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>> >>> >> >> >> > |
Nicolas Cellier wrote:
> Maybe #finalize could be sent directly by WeakKeyDictionary ? > WeakRegistry are protected forMutualExclusion, WeakKeyDictionary is not. > So WeakRegistry must stay. > There might be more subtle things you know better than me... > ... like finalizeValues is two folds: > 1) clean-up valueDictionary in a protected block > 2) finalize outside this block > Didn't get why... could #finalize have a chance to modify valueDictionary? No, but it could potentially take a long time and the idea was to not lock the entire registry while it's doing the finalization. The background is that you might be doing something like saying "socket close" where the close could take 20 secs. However, knowing what I know today I would argue that doing something like that isn't a valid approach to finalization anyway because it happens at high priority (if it takes twenty seconds then fork the damn finalizer!). So moving finalize into the critical secion (and into WeakKeyDictionary) is fine from my perspective. Cheers, - Andreas |
In reply to this post by Adrian Lienhard-2
I would change the symbol table from an open addressing weak set to a
hash bucketed weak set. Also, I'd take the opportunity to make sure the symbol table is thread safe. Adrian Lienhard wrote: > Hi, > > We have a server application that, after some longer usage, becomes > very slow. It turned out that this is because lots of symbols are > created from strings (without holding onto them), which becomes slower > and slower over time. This is easy to fix in our app, but I'd like to > understand why Squeak has a problem with that. > > Using the following snipped, I produced the attached chart (Y axis is > ms spent to create 5000 random symbols). You can nicely see the > reoccurring pattern that grows over time. > > 1000 timesRepeat: [ > Transcript print: ([ > 5000 timesRepeat: [ (($a to: $x) atRandom asString , 'asdasd') > asSymbol ] ] timeToRun); cr; flush. ] > > > > ------------------------------------------------------------------------ > |
In reply to this post by Nicolas Cellier
Thanks Nicolas!
As expected, the performance graph is constant with your fix. Your tests cover a lot of cases. Anything in particular one should test before integrating the fix? Cheers, Adrian On May 7, 2009, at 01:45 , Nicolas Cellier wrote: > Wait, there is a bug in scanFor: fixCollisionsFrom: does not recycle > the first nil and scanFor: then loops indefinitely :( see > http://bugs.squeak.org/view.php?id=7350 for update > > 2009/5/7 Andreas Raab <[hidden email]>: >> Nicolas Cellier wrote: >>> >>> Created http://bugs.squeak.org/view.php?id=7350 >> >> Thanks, this is great. Is this approach applicable to weak >> dictionaries as >> well? >> > > Should inquire, but it's getting late... > >> Cheers, >> - Andreas >> >>> 2009/5/6 Nicolas Cellier <[hidden email]>: >>>> >>>> Ah, I forgot to accept one change! >>>> It's even better if we decrease the tally when recycling a nil >>>> slot in >>>> #fixCollisionsFrom: >>>> >>>> Nicolas >>>> >>>> 2009/5/6 Nicolas Cellier <[hidden email]>: >>>>> >>>>> Though I have tried to attack it with some tricky combinations, >>>>> I did >>>>> not found a problem with current implementation. >>>>> It was funny to see how nil is handled as any other object in >>>>> WeakSet >>>>> fixCollisionsFrom: (scanFor: nil works). >>>>> >>>>> However, it's a pity not to recycle all those nils until next >>>>> grow! >>>>> Here comes a little trial for recycling reclaimed nil slots in >>>>> Pharo >>>>> (seems compatible with 3.10.2). >>>>> No guarantee... Note there is no WeakSet tests in Pharo nor 3.10.2 >>>>> currently :( >>>>> >>>>> Nicolas >>>>> >>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>> >>>>>> Nicolas Cellier wrote: >>>>>>> >>>>>>> Take this example: >>>>>> >>>>>> Ah, good point. (it works fine with o1 = 'g' and o2 = o3 = o4 = >>>>>> 'd'). >>>>>> The >>>>>> thing is that the test in WeakSet>>add: was misleading - it >>>>>> tests for >>>>>> nil >>>>>> where scanFor: searches only for flag. >>>>>> >>>>>> Cheers, >>>>>> - Andreas >>>>>> >>>>>>> ws := WeakSet new. >>>>>>> array := WeakSet new instVarAt: (WeakSet allInstVarNames >>>>>>> indexOf: >>>>>>> 'array'). >>>>>>> o1 hash \\ array size + 1 -> 1. >>>>>>> o2 hash \\ array size + 1 -> 2. >>>>>>> o3 hash \\ array size + 1 -> 1. >>>>>>> ws add: o1; add: o2; add: o3. >>>>>>> >>>>>>> o1 := o2 := nil. >>>>>>> Smalltalk garbageCollect. >>>>>>> "o1 and o2 are reclaimed." >>>>>>> (array at: 1) -> nil. >>>>>>> (array at: 2) -> nil. >>>>>>> >>>>>>> o4 hash \\ array size + 1 -> 1. >>>>>>> ws add o4. >>>>>>> (array at: 1) -> o4. >>>>>>> >>>>>>> (ws includes: o3) -> false. >>>>>>> >>>>>>> I will try and build a test case. Should be easy with Fractions. >>>>>>> >>>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>>> >>>>>>>> Nicolas Cellier wrote: >>>>>>>>> >>>>>>>>> Isn't that dangerous if you don't fixCollisionsFrom: index ? >>>>>>>> >>>>>>>> Why would it? All this does is avoiding to update the tally >>>>>>>> ivar if >>>>>>>> we're >>>>>>>> replacing a previously reclaimed entry with a new value. And >>>>>>>> I don't >>>>>>>> think >>>>>>>> that #fixCollisionsFrom: even looks at tally. >>>>>>>> >>>>>>>> Cheers, >>>>>>>> - Andreas >>>>>>>> >>>>>>>>> Nicolas >>>>>>>>> >>>>>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>>>>> >>>>>>>>>> Adrian Lienhard wrote: >>>>>>>>>>> >>>>>>>>>>> I haven't had time to look into this very closely yet but >>>>>>>>>>> maybe >>>>>>>>>>> somebody >>>>>>>>>>> else already knows what is going on. My guess is that the >>>>>>>>>>> NewSymbols >>>>>>>>>>> weak >>>>>>>>>>> set is repeatedly grown since tally is not decreased if >>>>>>>>>>> the GC >>>>>>>>>>> removes >>>>>>>>>>> a >>>>>>>>>>> weak element. >>>>>>>>>> >>>>>>>>>> That's an excellent guess. Try changing WeakSet to e.g., >>>>>>>>>> >>>>>>>>>> WeakSet>>atNewIndex: index put: aValue >>>>>>>>>> "Add newValue at the given index. Don't increment tally >>>>>>>>>> if current slot is nil since a previous value has been >>>>>>>>>> collected" >>>>>>>>>> (array at: index) ifNil:[^array at: index put: aValue]. >>>>>>>>>> ^super atNewIndex: index put: aValue >>>>>>>>>> >>>>>>>>>> Cheers, >>>>>>>>>> - Andreas >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>> >>> >> >> >> > |
2009/5/7 Adrian Lienhard <[hidden email]>:
> Thanks Nicolas! > > As expected, the performance graph is constant with your fix. > > Your tests cover a lot of cases. Anything in particular one should test > before integrating the fix? > > Cheers, > Adrian > Well, there is a possible infinite loop in #fixCollisionsFrom: - 1) This infinite loop can happen if set is full but full set never happens in "normal" condition unless some uncarefull programmer break contracts and use private messages - 2) This infinite loop was already there (but #fixCollisionsFrom: was used less frequently) - 3) This infinite loop is easy to avoid But as Andres suggested, the biggest problem might be concurrency issues... (several process changing/accessing a WeakSet concurrently). These issues did already exist, but are now more likely to happen statistically... - before the change, this was limited to concurrent write of WeakSet. - after the change, this can also happen for concurrent read This is because instance variable 'array' can be modified during the look-up phase (#scanFor:) if ever a hash-colliding entry was reclaimed near the accessed zone... Not the kind of issue easy to identify/debug/reproduce... Nicolas > On May 7, 2009, at 01:45 , Nicolas Cellier wrote: > >> Wait, there is a bug in scanFor: fixCollisionsFrom: does not recycle >> the first nil and scanFor: then loops indefinitely :( see >> http://bugs.squeak.org/view.php?id=7350 for update >> >> 2009/5/7 Andreas Raab <[hidden email]>: >>> >>> Nicolas Cellier wrote: >>>> >>>> Created http://bugs.squeak.org/view.php?id=7350 >>> >>> Thanks, this is great. Is this approach applicable to weak dictionaries >>> as >>> well? >>> >> >> Should inquire, but it's getting late... >> >>> Cheers, >>> - Andreas >>> >>>> 2009/5/6 Nicolas Cellier <[hidden email]>: >>>>> >>>>> Ah, I forgot to accept one change! >>>>> It's even better if we decrease the tally when recycling a nil slot in >>>>> #fixCollisionsFrom: >>>>> >>>>> Nicolas >>>>> >>>>> 2009/5/6 Nicolas Cellier <[hidden email]>: >>>>>> >>>>>> Though I have tried to attack it with some tricky combinations, I did >>>>>> not found a problem with current implementation. >>>>>> It was funny to see how nil is handled as any other object in WeakSet >>>>>> fixCollisionsFrom: (scanFor: nil works). >>>>>> >>>>>> However, it's a pity not to recycle all those nils until next grow! >>>>>> Here comes a little trial for recycling reclaimed nil slots in Pharo >>>>>> (seems compatible with 3.10.2). >>>>>> No guarantee... Note there is no WeakSet tests in Pharo nor 3.10.2 >>>>>> currently :( >>>>>> >>>>>> Nicolas >>>>>> >>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>> >>>>>>> Nicolas Cellier wrote: >>>>>>>> >>>>>>>> Take this example: >>>>>>> >>>>>>> Ah, good point. (it works fine with o1 = 'g' and o2 = o3 = o4 = 'd'). >>>>>>> The >>>>>>> thing is that the test in WeakSet>>add: was misleading - it tests for >>>>>>> nil >>>>>>> where scanFor: searches only for flag. >>>>>>> >>>>>>> Cheers, >>>>>>> - Andreas >>>>>>> >>>>>>>> ws := WeakSet new. >>>>>>>> array := WeakSet new instVarAt: (WeakSet allInstVarNames indexOf: >>>>>>>> 'array'). >>>>>>>> o1 hash \\ array size + 1 -> 1. >>>>>>>> o2 hash \\ array size + 1 -> 2. >>>>>>>> o3 hash \\ array size + 1 -> 1. >>>>>>>> ws add: o1; add: o2; add: o3. >>>>>>>> >>>>>>>> o1 := o2 := nil. >>>>>>>> Smalltalk garbageCollect. >>>>>>>> "o1 and o2 are reclaimed." >>>>>>>> (array at: 1) -> nil. >>>>>>>> (array at: 2) -> nil. >>>>>>>> >>>>>>>> o4 hash \\ array size + 1 -> 1. >>>>>>>> ws add o4. >>>>>>>> (array at: 1) -> o4. >>>>>>>> >>>>>>>> (ws includes: o3) -> false. >>>>>>>> >>>>>>>> I will try and build a test case. Should be easy with Fractions. >>>>>>>> >>>>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>>>> >>>>>>>>> Nicolas Cellier wrote: >>>>>>>>>> >>>>>>>>>> Isn't that dangerous if you don't fixCollisionsFrom: index ? >>>>>>>>> >>>>>>>>> Why would it? All this does is avoiding to update the tally ivar if >>>>>>>>> we're >>>>>>>>> replacing a previously reclaimed entry with a new value. And I >>>>>>>>> don't >>>>>>>>> think >>>>>>>>> that #fixCollisionsFrom: even looks at tally. >>>>>>>>> >>>>>>>>> Cheers, >>>>>>>>> - Andreas >>>>>>>>> >>>>>>>>>> Nicolas >>>>>>>>>> >>>>>>>>>> 2009/5/6 Andreas Raab <[hidden email]>: >>>>>>>>>>> >>>>>>>>>>> Adrian Lienhard wrote: >>>>>>>>>>>> >>>>>>>>>>>> I haven't had time to look into this very closely yet but maybe >>>>>>>>>>>> somebody >>>>>>>>>>>> else already knows what is going on. My guess is that the >>>>>>>>>>>> NewSymbols >>>>>>>>>>>> weak >>>>>>>>>>>> set is repeatedly grown since tally is not decreased if the GC >>>>>>>>>>>> removes >>>>>>>>>>>> a >>>>>>>>>>>> weak element. >>>>>>>>>>> >>>>>>>>>>> That's an excellent guess. Try changing WeakSet to e.g., >>>>>>>>>>> >>>>>>>>>>> WeakSet>>atNewIndex: index put: aValue >>>>>>>>>>> "Add newValue at the given index. Don't increment tally >>>>>>>>>>> if current slot is nil since a previous value has been collected" >>>>>>>>>>> (array at: index) ifNil:[^array at: index put: aValue]. >>>>>>>>>>> ^super atNewIndex: index put: aValue >>>>>>>>>>> >>>>>>>>>>> Cheers, >>>>>>>>>>> - Andreas >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>> >>>> >>> >>> >>> >> > > > |
Free forum by Nabble | Edit this page |