pharo icon indicating copy to clipboard operation
pharo copied to clipboard

Making allocations through a dynamic variable

Open jordanmontt opened this issue 1 year ago • 4 comments

This PR introduced a DynamicVariable to make the allocations instead of directly calling basicNew. The reason behind this is that now Pharo has 3 allocator methods (new, newPinnned, and newTenured). Like described in issue #16731 there are around 154 re-definitions of the method new. This causes problems since if the user calls another allocator method for those classes that have re-defined new, the object will be not be initialized properly. Fixes #16731.

This PR changes the way allocations are made by using a DynamicVariable. Now, no matter will allocator method is called, the objects will be always be properly initialized even when re-defining the new method.

Let's take OrderedCollection as an example. Currently in Pharo, if one does:

OrderedCollection newPinned add: 1 this will raised an exception since the OrderedCollection object was not initialized because of the re-definition of new.

With this PR: doing OrderedCollection newPinned add: 1 or even OrderedCollection newTenured add: 1 works as excepted.


About performance

I ran some performance benches and this PR increases the allocation time.

"WITH allocator dynamic variable"
[ Object new ] bench. "54,359,190 iterations in 5 seconds 3 milliseconds. 10865318.809 per second"

[ OrderedCollection new ] bench. "51,635,208 iterations in 5 seconds 1 millisecond. 10324976.605 per second"

[ Array new: 10 ] bench "324,588,991 iterations in 5 seconds 1 millisecond. 64904817.237 per second"
"WITHOUT allocator dynamic variable"

[ Object new ] bench. "127,249,785 iterations in 5 seconds 2 milliseconds. 25439781.088 per second"

[ OrderedCollection new ] bench. "94,225,669 iterations in 5 seconds 1 millisecond. 18841365.527 per second"

[ Array new: 10 ] bench "457,925,147 iterations in 5 seconds 2 milliseconds. 91548410.036 per second"

So, this means that making the allocation through the DynamicVariable reduces performance:

  • Around 2.34 times slower for Object new
  • Around 1.8 times slower for OrderedCollection new
  • Around 1.4 times slower for Array new: 10

jordanmontt avatar Jun 03 '24 10:06 jordanmontt

Another considerations:

  • This change will alter the scopes of the allocation. That means, if one allocates an OrderedCollection with newTenured, all the allocations that will be produced during the allocation of the collection will be tenured too; not only the OrderedCollection object itself.
  • This PR breaks the bootstrapping process
  • It would be nice to have some solution that only penalizes the allocations made by a different allocator (and not the ones made with basicNew). With this PR, we make all the allocations slower. One solution for this can be to change the primitive basicNew to check with allocator is used. If is the normal one, proceed normally; if it is another one (like tenured) fail the primitive and execute the fallback code

jordanmontt avatar Jun 03 '24 14:06 jordanmontt

This is a nice change but I fear the degraded performance of the classical new will be a no go! for integration. I agree that having a solution that only penalizes the allocations made by a different allocator would be nice. Did you give a try to see if it is possible to optimize the classical new way? If not possible, maybe this feature could be packaged separately so that only users that need the feature will be impacted by performance ?

demarey avatar Jun 05 '24 06:06 demarey

@jordanmontt have you measured the impact in performance?

tesonep avatar Jun 05 '24 18:06 tesonep

What I was thinking is could not we have a special VM or library when we want to do such analyses?

Ducasse avatar Jun 05 '24 20:06 Ducasse