MCPackageLoader>>#basicLoadDefinitions loads classes twice

[chisandrei]

The method MCPackageLoader>>#basicLoadDefinitions loads classes twice. This is mentioned in its comment. However, the fact that classes is loaded twice leads to a significant slowdown especially in code that uses traits.

The screenshot above shows a profiler on loading GT. loadClassDefinition: which is the first pass to load classes and traits takes 7.3% of the time. Then tryToLoad: which should load methods also loads again classes. This time loading the same class definitions as before takes 30% of the time!!!

If we expand we see that most time is spend in the TraitBuilderEnhancher. For example when migrating class instances Behaviour class >>#allInstances is now called a lot, taking a significant amount of time.

I'm not sure if there are any side-effects but loading also class definitions while loading method definitions should not be necessary. One maybe naive solution is to have tryToLoad: as below. It does not attempt to load again class definitions.

tryToLoad: aDefinition
	aDefinition isClassDefinition ifTrue: [ ^ false ].
		
	[aDefinition addMethodAdditionTo: methodAdditions] on: Error do: [errorDefinitions add: aDefinition].'

I tried the above solution on loading the following code in a Pharo 9 image:

Time millisecondsToRun: [ 
	Metacello new
        repository: 'github://j-brant/smacc';
        baseline: 'SmaCC';
        load.

	Metacello new
        repository: 'github://PolyMathOrg/PolyMath';
        baseline: 'PolyMath';
        load.
	
	Metacello new
 		baseline:'Seaside3';
 		repository: 'github://SeasideSt/Seaside:master/repository';
 		load.
		
	Metacello new
    	baseline: 'PetitParser2';
    	repository: 'github://kursjan/petitparser2';
    	load.
	
	Metacello new
  		baseline: 'Moose';
  		repository: 'github://moosetechnology/Moose:development/src';
  		load.
 ] 

In a default image this takes around 681086ms. With the above change it takes 473210ms. So a lot faster!

If I run RPackageOrganizer default packages sumNumbers: #linesOfCode I get 1137393 lines of code in the default image and 1137394 in the one with the change. Not sure why the 1 line of code difference, but it's in the Monticello package.

Pharo9.0.0
Build information: Pharo-9.0.0+build.1000.sha.f92e709a70b5c621d55833be4ae6c310fd64e612 (64 Bit)
Unnamed

CoInterpreter VMMaker-tonel.1 uuid: ae104fe7-6171-0d00-8750-f11d03f63bb3 Nov 26 2020
StackToRegisterMappingCogit VMMaker-tonel.1 uuid: ae104fe7-6171-0d00-8750-f11d03f63bb3 Nov 26 2020
cd129301 - Commit: cd129301 - Date: 2020-11-26 11:38:10 +0100

Pharo 9.0.0 built on Nov 26 2020 11:41:49 Compiler: 4.2.1 Compatible Apple LLVM 11.0.3 (clang-1103.0.32.29)
VMMaker versionString cd129301 - Commit: cd129301 - Date: 2020-11-26 11:38:10 +0100
CoInterpreter VMMaker-tonel.1 uuid: ae104fe7-6171-0d00-8750-f11d03f63bb3 Nov 26 2020
StackToRegisterMappingCogit VMMaker-tonel.1 uuid: ae104fe7-6171-0d00-8750-f11d03f63bb3 Nov 26 2020

[MarcusDenker]

It seems that the only thing not working is related to slot definitions... the bootstrapped image has tests failing in SpObservableSlotTest.

(this is odd as the undefined slot should normally take care of this... to be investigated)