Update MethodDictionary rebuild to speed up trait recompilation by jecisc · Pull Request #19870 · pharo-project/pharo

jecisc · 2026-07-08T11:15:10Z

This PR brings multiple speed up for large trait recompilation.

Recompiling large traits can be really slow because it needs to rebuild the method dictionary of each of the trait user.

This was done by iterating over the selectors of the trait composition and installing a copy of the method associated in the user. But we traversed multiple time the trait composition by doing this.

Now I build a compilation info object containing all the info directly preventing multiple visits of the trait composition.

Most of the time we still spend in the rebuild is spend in recompiling the methods if the user has a slot. The installation time of the method is much shorter.

For comparison, in a vanilla Moose image it was taking 72sec to add a slot to TEntityMetaLevelDependency. With this change it drops to 27sec.

There might be ways to speed up a little more operations, I might explore this later.

…e way to reject some selectors

For now one is failing

…from a trait

…a method

Gabriel-Darbord

Thanks for this PR, this is great for Moose where traits are omnipresent :)
I have a few technical and quality suggestions.
I can't really comment on semantics and validity, but I don't see any blatant issue there.

Gabriel-Darbord · 2026-07-09T07:07:50Z

+	| innerInfos aliassed |
+	"I calculate the new aliases selectors"
+	innerInfos := inner methodsCompilationInfo.
+	aliassed := OrderedCollection new: self aliases size.
+
+	self aliases associations do: [ :association |
+			innerInfos
+				detect: [ :info | info selector = association value ]
+				ifFound: [ :info |
+						aliassed add: (TraitMethodCompilationInfo new
+								 selector: association key;
+								 definingTrait: info definingTrait;
+								 compiledMethod: info compiledMethod;
+								 sourceCode: (info compiledMethod getSourceReplacingSelectorWith: association key);
+								 yourself) ] ].
+
+	^ innerInfos , aliassed


aliassed -> aliased

Suggested change

| innerInfos aliassed |

"I calculate the new aliases selectors"

innerInfos := inner methodsCompilationInfo.

aliassed := OrderedCollection new: self aliases size.

self aliases associations do: [ :association |

innerInfos

detect: [ :info | info selector = association value ]

ifFound: [ :info |

aliassed add: (TraitMethodCompilationInfo new

selector: association key;

definingTrait: info definingTrait;

compiledMethod: info compiledMethod;

sourceCode: (info compiledMethod getSourceReplacingSelectorWith: association key);

yourself) ] ].

^ innerInfos , aliassed

| innerInfos aliased |

"I calculate the new aliases selectors"

innerInfos := inner methodsCompilationInfo.

aliased := OrderedCollection new: self aliases size.

self aliases associations do: [ :association |

innerInfos

detect: [ :info | info selector = association value ]

ifFound: [ :info |

aliased add: (TraitMethodCompilationInfo new

selector: association key;

definingTrait: info definingTrait;

compiledMethod: info compiledMethod;

sourceCode: (info compiledMethod getSourceReplacingSelectorWith: association key);

yourself) ] ].

^ innerInfos , aliased

Gabriel-Darbord · 2026-07-09T07:08:25Z

+	innerInfos := inner methodsCompilationInfo.
+	aliassed := OrderedCollection new: self aliases size.
+
+	self aliases associations do: [ :association |


Suggested change

self aliases associations do: [ :association |

self aliases associationsDo: [ :association |

Gabriel-Darbord · 2026-07-09T08:40:04Z

-			TraitedClass allSlots) anySatisfy: [ :x | x name = e name ] ]
+
+	| slotNamesToReject |
+	"Save in a variable to not recomcatenate for each slot"


Suggested change

"Save in a variable to not recomcatenate for each slot"

"Save in a variable to not re-concatenate for each slot"

Gabriel-Darbord · 2026-07-09T08:47:57Z

+	| innerInfos traitInfos localSelectors methods |
+	"We first collect the compilation info for the local methods I have."
+	innerInfos := innerClass localMethods collect: [ :method |
+			              TraitMethodCompilationInfo new
+				              selector: method selector;
+				              definingTrait: self;
+				              compiledMethod: method;
+				              sourceCode: method sourceCode;
+				              yourself ].
+
+	"If I need to alias the talent initializer, I do it now."
+	innerInfos do: [ :info |
+			info selector = #initializeTalent ifTrue: [
+					info
+						selector: self initializeSelectorForMe;
+						sourceCode: (info compiledMethod getSourceReplacingSelectorWith: self initializeSelectorForMe) ] ].
+


Memoize self initializeSelectorForMe since it's re-computed each time by TaCompositionElement?

Suggested change

| innerInfos traitInfos localSelectors methods |

"We first collect the compilation info for the local methods I have."

innerInfos := innerClass localMethods collect: [ :method |

TraitMethodCompilationInfo new

selector: method selector;

definingTrait: self;

compiledMethod: method;

sourceCode: method sourceCode;

yourself ].

"If I need to alias the talent initializer, I do it now."

innerInfos do: [ :info |

info selector = #initializeTalent ifTrue: [

info

selector: self initializeSelectorForMe;

sourceCode: (info compiledMethod getSourceReplacingSelectorWith: self initializeSelectorForMe) ] ].

| innerInfos selector traitInfos localSelectors methods |

"We first collect the compilation info for the local methods I have."

innerInfos := innerClass localMethods collect: [ :method |

TraitMethodCompilationInfo new

selector: method selector;

definingTrait: self;

compiledMethod: method;

sourceCode: method sourceCode;

yourself ].

"If I need to alias the talent initializer, I do it now."

selector := self initializeSelectorForMe.

innerInfos do: [ :info |

info selector = #initializeTalent ifTrue: [

info

selector: selector;

sourceCode: (info compiledMethod getSourceReplacingSelectorWith: selector) ] ].

Gabriel-Darbord · 2026-07-09T08:57:57Z

+	| preferedInfos |
+	preferedInfos := self preferedTrait methodsCompilationInfo.
+
+	implementations
+		detect: [ :info | preferedInfos includes: info ]


prefered -> preferred, but changing preferedTrait is outside the current scope of this PR.

Suggested change

| preferedInfos |

preferedInfos := self preferedTrait methodsCompilationInfo.

implementations

detect: [ :info | preferedInfos includes: info ]

| preferredInfos |

preferredInfos := self preferedTrait methodsCompilationInfo.

implementations

detect: [ :info | preferredInfos includes: info ]

Gabriel-Darbord · 2026-07-09T12:45:25Z

-	| selectors removedSelectors modified |
+	| compilationInfos removedSelectors modified |
 	"During the creation of the class or after a change in the traitComposition, the whole method dictionary is calculated.
 	If I return true, my users should be updated""1. I recreate the local methodDict"


Consider formatting this method?
Also these comments have been unintentionally merged previously:

Suggested change

If I return true, my users should be updated""1. I recreate the local methodDict"

If I return true, my users should be updated"

"1. I recreate the local methodDict"

Gabriel-Darbord · 2026-07-10T07:36:18Z

+	removedSelectors := self methodDict keys reject: [ :aSelector |
+		                    (compilationInfos anySatisfy: [ :compilationInfo | compilationInfo selector = aSelector ]) or: [ self isSelectorToKeep: aSelector ] ].
+	modified := modified | removedSelectors isNotEmpty.
 	removedSelectors do: [ :aSelector |
-		self methodDict removeKey: aSelector.
-		self removeFromProtocols: aSelector  ].
+			self methodDict removeKey: aSelector.
+			self removeFromProtocols: aSelector ].


removedSelectors seems like an unnecessary intermediate collection:

self methodDict keys do: [ :aSelector | ((compilationInfos anySatisfy: [ :compilationInfo | compilationInfo selector = aSelector ]) or: [ self isSelectorToKeep: aSelector ]) ifTrue: [ modified := true. self methodDict removeKey: aSelector. self removeFromProtocols: aSelector ] ].

Explicitly use keys do: instead of keysDo: to avoid modification during iteration.
You could also try swapping conditions to check isSelectorToKeep: first to be potentially faster.

Gabriel-Darbord · 2026-07-10T07:43:09Z

+	self == anObject ifTrue: [ ^ true ].
+	self class = anObject class ifFalse: [ ^ false ].
+	^ selector = anObject selector and: [
+		  definingTrait = anObject definingTrait and: [ sourceCode = anObject sourceCode and: [ self protocol = anObject protocol ] ] ]


Protocol comparison is faster than sourceCode comparison and we want to fail-fast:

Suggested change

definingTrait = anObject definingTrait and: [ sourceCode = anObject sourceCode and: [ self protocol = anObject protocol ] ] ]

definingTrait = anObject definingTrait and: [ self protocol = anObject protocol and: [ sourceCode = anObject sourceCode ] ] ]

Gabriel-Darbord · 2026-07-10T07:47:03Z

+	super printOn: aStream.
+
+	aStream
+		nextPutAll: ' [ ';


More in-line with other printOn implementation, do not include a space after the bracket, though the space before is up to you (personally I prefer it compact).

Suggested change

nextPutAll: ' [ ';

nextPutAll: '[';

Gabriel-Darbord · 2026-07-10T07:55:07Z

+		nextPutAll: '>>';
+		print: selector.
+
+	self compiledMethod ifNotNil: [ :cm | cm isRequired ifTrue: [ aStream nextPutAll: ' - Explicit requirement' ] ].


If memoized and not afraid of concurrency (compiledMethod could change between nil check and block execution, but I don't think that's a concern for this data structure): cm isRequired -> self isExplicitRequirement

jecisc added 30 commits June 15, 2026 23:14

Init a speedup of traits

bae2584

Start to implement new compilation info collector

f7b4727

Yet another compilation info implementation

4933195

Finish to add compilation info that save only selectors + simplify th…

a214d86

…e way to reject some selectors

Begin to get the defining trait from the compilation info

1c07f21

Aliases shuld provide the defining trait

47c7143

Rely solely on compilation info to get the defining trait

e56b3da

Use compilation info to get the compiled method

ca29ddf

Merge branch 'Pharo14' into kernel/trait-recompilation

5bbb24d

Use compilation info in doRebuildMethodDictionary

bf1f4b8

Manage protocol via compilation info

970a800

Begin to add tests of TraitMethodToCompileInfo

506ca02

For now one is failing

Fix test diamond

28226d6

Merge branch 'Pharo14' into kernel/trait-rebuild

959d8ec

Improve tests

6a27ed7

Add failing tests

3a91e46

Renaming

cf7b49b

Make tests pass

d888bcd

Do not depend on Moose

a0ca761

Same as previous, remove dependency to Moose

756be88

Merge branch 'Pharo14' into kernel/trait-rebuild

47299b1

Use compilation info to get the source code of a method to recompile …

58c8212

…from a trait

Update temp names

e7462e1

Use compilation info to update selectors in trait changes

3cd19c1

Try to use compilation infos to remove selectors from trait

ef968b1

Remove last user of selector installation in traits

6d29549

Remove dead code

f844a21

Start to use compile info for copying methods of trait composition

d1d1e9f

Remove one more user of selectors

4610266

Fix bug in defining trait selection

5a75b84

jecisc added 11 commits June 27, 2026 03:03

Use defining trait from the compilation info to copy

1bc183e

smallt cleanup

b35149c

Merge branch 'kernel/clean-traits' into kernel/trait-rebuild

2628fbd

Fix a bug in creation of compilation info

b6b6b51

Finish to use compilation info to copy methods

e461847

Merge branch 'traits/protocol-removal' into kernel/trait-rebuild

566ca9b

Merge branch 'Pharo14' into kernel/trait-rebuild

018da0e

Use the protocol of the compilation info

d3bb18d

Use compilation info to know if we need to update the source code of …

be7d4bd

…a method

Speed up knowing if a trait composition has slots

2309327

documentation

7435f73

jecisc added the Status: Tests passed please review! label Jul 8, 2026

Gabriel-Darbord reviewed Jul 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Update MethodDictionary rebuild to speed up trait recompilation#19870

Update MethodDictionary rebuild to speed up trait recompilation#19870
jecisc wants to merge 41 commits into
pharo-project:Pharo14from
jecisc:kernel/trait-rebuild

jecisc commented Jul 8, 2026

Uh oh!

Gabriel-Darbord left a comment

Uh oh!

Gabriel-Darbord Jul 9, 2026

Uh oh!

Gabriel-Darbord Jul 9, 2026

Uh oh!

Gabriel-Darbord Jul 9, 2026

Uh oh!

Gabriel-Darbord Jul 9, 2026

Uh oh!

Gabriel-Darbord Jul 9, 2026

Uh oh!

Gabriel-Darbord Jul 9, 2026

Uh oh!

Gabriel-Darbord Jul 10, 2026

Uh oh!

Gabriel-Darbord Jul 10, 2026

Uh oh!

Gabriel-Darbord Jul 10, 2026

Uh oh!

Gabriel-Darbord Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	self aliases associations do: [ :association \|
	self aliases associationsDo: [ :association \|

	"Save in a variable to not recomcatenate for each slot"
	"Save in a variable to not re-concatenate for each slot"

	If I return true, my users should be updated""1. I recreate the local methodDict"
	If I return true, my users should be updated"
	"1. I recreate the local methodDict"

	definingTrait = anObject definingTrait and: [ sourceCode = anObject sourceCode and: [ self protocol = anObject protocol ] ] ]
	definingTrait = anObject definingTrait and: [ self protocol = anObject protocol and: [ sourceCode = anObject sourceCode ] ] ]

Uh oh!

Uh oh!

Conversation

jecisc commented Jul 8, 2026

Uh oh!

Gabriel-Darbord left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants