Skip to content

Conversation

rolandbernard
Copy link

@rolandbernard rolandbernard commented Oct 10, 2025

We (Lifeware) have some issues in Pharo with the performance of become:. This hurts us in some code that performs a very large number of calls to become:. The following code illustrates the issue:

Time millisecondsToRun:
        [| a b |
        a := (1 to: 1000) collect: [:e | ValueHolder new].
        b := (1 to: a size) collect: [:e | ValueHolder new].
        a with: b do: [:a1 :a2 | a1 become: a2]]

In vanilla Pharo it takes about 20 milliseconds on my machine. In a Pharo image with GT + Lifeware code (resulting in ~175000 classes) it takes about 1 second.

We found that the issue is that in the Pharo VM, become: scans through the complete class table (nearly) every time. This means the more classes there are in the image, the slower it will be. Note that the method comment in SpurMemoryManager>>#postBecomeScanClassTable: indicates that this should only happen when an active class object has been becomed.

In this pull request I try to make the change to only do the scan in case we actually becomed an active class object by changing the guard slightly. With this it only takes 7 milliseconds in the image with GT + Lifeware code (and 3 milliseconds in vanilla Pharo). While I think that should be correct, because it also aligns with the comment in the method I changed, this should definitely be reviewed by someone with more knowledge about the VM than me.

@Ducasse
Copy link
Member

Ducasse commented Oct 10, 2025

Just out of curiosity what the users of become: in your image.

If I remember correctly become: does not scan the complete memory but place forwarders that are removed during the next GC phase.

@Ducasse
Copy link
Member

Ducasse commented Oct 10, 2025

@tesonep @guillep what is an active class?

@PalumboN
Copy link
Collaborator

If I remember correctly become: does not scan the complete memory but place forwarders that are removed during the next GC phase.

The is true, but the thing here is that it also scan the class table, to avoid having forwarders there?

@tesonep @guillep what is an active class?

I imagine it is a class in the class table?? 🤷‍♂️

@rolandbernard
Copy link
Author

Just out of curiosity what the users of become: in your image.

We have many different users in our image, but the one that made us aware of this issue involves lazy copying of objects (i.e., creating a dummy and copying the object when looked at, using become: to replace the dummy).

what is an active class?

I imagine it is a class in the class table?? 🤷‍♂️

This is also what I understand from SpurMemoryManager>>#becomeEffectFlagsFor: at least.

SpurMemoryManager >> becomeEffectFlagsFor: objOop [
"Answer the appropriate become effect flags for objOop, or 0 if none.
The effect flags determine how much work is done after the become
in following forwarding pointers, voiding method caches, etc."
<inline: false>
^(self isPointersNonImm: objOop)
ifTrue:
[| hash |
((hash := self rawHashBitsOf: objOop) ~= 0
and: [(self classAtIndex: hash) = objOop])
ifTrue: [BecamePointerObjectFlag + BecameActiveClassFlag]
ifFalse: [BecamePointerObjectFlag]]
ifFalse:
[(self isCompiledMethod: objOop)
ifTrue: [BecameCompiledMethodFlag]
ifFalse: [0]]
]

@VincentBlondeau VincentBlondeau moved this to Needs triage in Pharo Issues Oct 17, 2025
@VincentBlondeau VincentBlondeau moved this from Needs triage to High priority in Pharo Issues Oct 17, 2025
@tesonep
Copy link
Collaborator

tesonep commented Oct 17, 2025

Hi, this PR looks promising. We should make it against pharo-12 branch so it can be tested in the dev version of the VM

@rolandbernard rolandbernard changed the base branch from pharo-10 to pharo-12 October 17, 2025 13:42
@rolandbernard rolandbernard changed the base branch from pharo-12 to pharo-10 October 17, 2025 13:42
@rolandbernard rolandbernard changed the base branch from pharo-10 to pharo-12 October 17, 2025 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants