Issue #913: ~40x performance improvement over old implementation of R…

…wPrjBrowserToolV2 >>#addOrUpdateMethod:dictionaries:inProtocol:forClassNamed:isMeta:inPackageNamed:
GemTalk · Jan 20, 2024 · 9c0c9b0 · 9c0c9b0
1 parent 72a8d6d
commit 9c0c9b0
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 48 deletions.
diff --git a/rowan/src/Rowan-GemStone-CoreV2/RwGsLoadedSymbolDictResolvedProjectV2.class.st b/rowan/src/Rowan-GemStone-CoreV2/RwGsLoadedSymbolDictResolvedProjectV2.class.st
@@ -35,6 +35,11 @@ RwGsLoadedSymbolDictResolvedProjectV2 >> _projectDefinitionPlatformConditionalAt
 	^ self resolvedProject _projectDefinitionPlatformConditionalAttributes: platformConditionalAttributes
 ]
 
+{ #category : 'accessing' }
+RwGsLoadedSymbolDictResolvedProjectV2 >> _projectSpecification [
+	^ handle _projectSpecification
+]
+
 { #category : 'private' }
 RwGsLoadedSymbolDictResolvedProjectV2 >> _requiredProjectNamesForLoadedProject: visitedSet [
 	^ self handle _requiredProjectNamesForLoadedProject: visitedSet
@@ -222,6 +227,11 @@ RwGsLoadedSymbolDictResolvedProjectV2 >> componentsRoot [
 	^ self resolvedProject componentsRoot
 ]
 
+{ #category : 'accessing' }
+RwGsLoadedSymbolDictResolvedProjectV2 >> componentsWithDoits [
+	^ self _projectComponents componentsWithDoits
+]
+
 { #category : 'accessing' }
 RwGsLoadedSymbolDictResolvedProjectV2 >> conditionalAttributes [
 
@@ -251,12 +261,23 @@ RwGsLoadedSymbolDictResolvedProjectV2 >> gemstoneDefaultSymbolDictName [
 	^ self resolvedProject gemstoneDefaultSymbolDictName
 ]
 
+{ #category : 'querying' }
+RwGsLoadedSymbolDictResolvedProjectV2 >> gemstoneMethodEnvForPackageNamed: packageName [
+
+	^ handle gemstoneMethodEnvForPackageNamed: packageName
+]
+
 { #category : 'accessing' }
 RwGsLoadedSymbolDictResolvedProjectV2 >> gemstoneSymbolDictNameForPackageNamed: packageName [
 
 	^self resolvedProject gemstoneSymbolDictNameForPackageNamed: packageName
 ]
 
+{ #category : 'accessing' }
+RwGsLoadedSymbolDictResolvedProjectV2 >> gemstoneUseSessionMethodsForExtensionsForPackageNamed: packageName [
+	^ handle gemstoneUseSessionMethodsForExtensionsForPackageNamed: packageName
+]
+
 { #category : 'accessing' }
 RwGsLoadedSymbolDictResolvedProjectV2 >> gitRoot [
 	^ self resolvedProject gitRoot

diff --git a/rowan/src/Rowan-Tests/RwBrowserTool48991Test.class.st b/rowan/src/Rowan-Tests/RwBrowserTool48991Test.class.st
@@ -28,7 +28,8 @@ RwBrowserTool48991Test >> testAddMethod [
 
 	browserTool := RwPrjBrowserTool new.
 
-	browserTool
+	"add new method"
+	gsNMethod := browserTool
 		addOrUpdateMethod: 'ivar1: anInteger ivar1 := anInteger'
 		inProtocol: 'accessing'
 		forClassNamed: className
@@ -42,7 +43,15 @@ RwBrowserTool48991Test >> testAddMethod [
 	testInstance perform: #'ivar1:' with: 3.
 	self assert: (testInstance perform: #'ivar1') = 3.
 	self should: [ testInstance perform: #'foo' ] raise: MessageNotUnderstood.
+	self assert: testClass rowanProjectName = projectName.
+	self assert: testClass rowanPackageName = packageName1.
+
+	compiledMethod := testClass compiledMethodAt: #'ivar1:'.
+	self assert: gsNMethod == compiledMethod.
+	self assert: compiledMethod rowanProjectName = projectName.
+	self assert: compiledMethod rowanPackageName = packageName1.
 
+	"add new extension method"
 	browserTool
 		addOrUpdateMethod: 'foo ^''bar'''
 		inProtocol: '*' , packageName2 asLowercase

diff --git a/rowan/src/Rowan-Tools-Core/RwPrjBrowserTool.class.st b/rowan/src/Rowan-Tools-Core/RwPrjBrowserTool.class.st
@@ -261,7 +261,7 @@ RwPrjBrowserTool >> addOrUpdateMethod: methodSource dictionaries: aSymbolList in
 		to a class extension in the named package.
 	 Return the resulting compiled method"
 
-	| beforeMethod afterMethod classDefinition methodModification classModification theClassName instanceMethodsModification packageDefinition thePackageName packageModification classesModification projectDefinition theProjectName projectModification packagesModification 
+	| beforeMethod afterMethod classDefinition methodModification classModification packageDefinition packageModification classesModification projectModification packagesModification loadedProjects
 
 "-------------------------------------------------"
 	  fromLoadedPackage fromLoadedProject 
@@ -271,9 +271,6 @@ RwPrjBrowserTool >> addOrUpdateMethod: methodSource dictionaries: aSymbolList in
 	methodsModification
 |
 
-	theClassName := className.
-	thePackageName := packageName.
-
 "-------------------------------------------------"
 
 	afterMethod := RwMethodDefinition newForSource: methodSource protocol: protocol.
@@ -302,8 +299,6 @@ RwPrjBrowserTool >> addOrUpdateMethod: methodSource dictionaries: aSymbolList in
 				beforeMethod := fromLoadedMethod asDefinition.
 				 ].
 
-	theLoadedClass loadedPackage name = packageName 
-		ifFalse: [ self error: 'Not handling extension methods for classes, yet' ].
 	toLoadedPackage name = packageName 
 		ifFalse: [ self error: 'Not handling method moves between packages, yet' ].
 	fromLoadedProject 
@@ -312,10 +307,20 @@ RwPrjBrowserTool >> addOrUpdateMethod: methodSource dictionaries: aSymbolList in
 				ifFalse: [ self error: 'Not handling method moves between projects, yet' ] ].
 
 	classDefinition := theLoadedClass asDefinition.
-	methodModification := (afterMethod compareAgainstBase: beforeMethod).
+
+	methodModification := theLoadedClass loadedPackage name = packageName 
+		ifTrue: [ afterMethod compareAgainstBase: beforeMethod ]
+		ifFalse: [ afterMethod compareExtensionMethodsAgainstBase: beforeMethod ].
+	methodModification
+		isMeta: isMeta;
+		classDefinition: classDefinition;
+		yourself.
 "
 	ENTERING THE FABRICATION ZONE
+		1. _projectDefinitionPlatformConditionalAttributes is not being handled
+		2. gs_symbolDictionary: for packages and class defs is not being done either
 "
+
 	projectModification := RwProjectModification 
 		before: toLoadedProject
 		after: toLoadedProject.
@@ -340,47 +345,14 @@ RwPrjBrowserTool >> addOrUpdateMethod: methodSource dictionaries: aSymbolList in
 	packageModification isEmpty
 		ifFalse: [ packagesModification addElementModification: packageModification ].
 	projectModification packagesModification: packagesModification.
-"
-	ENTERING compareAgainstBaseForLoader: ZONE
-"
 	projectSetModification := RwProjectSetModification new.
-"
-	self
-		compareDictionary: dictionaryOfProjectDefinitions
-		againstBaseDictionary: dictionaryOfProjectDefinitions
-		into: result
-		elementClass: RwProjectDefinition.
-"
-"-------------------------------------------------"
-self halt.
-"-------------------------------------------------"
-
-"-------------------------------------------------"
-self halt: 'not expected to be run'.
-"-------------------------------------------------"
-
-
-	packageDefinition := RwPackageDefinition newNamed: thePackageName.
-
-	projectDefinition := RwProjectDefinition new
-		projectName: theProjectName;
-		yourself.
-
-	projectModification := projectDefinition compareAgainstBase: projectDefinition.
-	packagesModification := RwPackagesModification new.
-	packageModification := packageDefinition compareAgainstBase: packageDefinition.
-	classesModification := RwClassesModification new.
-	classModification := classDefinition compareAgainstBase: classDefinition.
-	instanceMethodsModification := RwMethodsModification
-		extendedClassName: theClassName.
-
-	methodModification := (afterMethod compareAgainstBase: beforeMethod)
-		isMeta: false;
-		classDefinition: classDefinition;
-		yourself.
-
-
-	projectModification halt
+	projectModification isEmpty
+		ifFalse: [ projectSetModification addElementModification: projectModification ].
+	loadedProjects := Rowan projectTools load
+		loadProjectSetModification: projectSetModification
+		symbolList: aSymbolList.
+	^ (self _loadedMethod: afterMethod selector inClassNamed: className isMeta: isMeta)
+		handle
 ]
 
 { #category : 'method browsing' }

diff --git a/rowan/src/Rowan-Tools-CoreV2/RwPrjLoadToolV2.class.st b/rowan/src/Rowan-Tools-CoreV2/RwPrjLoadToolV2.class.st
@@ -117,6 +117,17 @@ RwPrjLoadToolV2 >> _doProjectSetLoad: projectSetDefinition instanceMigrator: ins
 		processedClassNames: processedClassNames
 ]
 
+{ #category : 'private' }
+RwPrjLoadToolV2 >> _doProjectSetModificationLoad: projectSetModification instanceMigrator: instanceMigrator symbolList: symbolList processedClassNames: processedClassNames [
+	"Wrapper method in case we need to handle RwExistingVisitorAddingExistingClassNotification RwExistingVisitorAddingExistingMethodNotification here ... 
+		currently this method is used for single method/class updates so we should be able to handle these in the process of create projectSetModification"
+
+	^ self
+		_loadProjectSetModification: projectSetModification
+		instanceMigrator: instanceMigrator
+		symbolList: symbolList
+]
+
 { #category : 'private' }
 RwPrjLoadToolV2 >> _loadProjectDefinition: projectDefinition customConditionalAttributes: customConditionalAttributes instanceMigrator: instanceMigrator symbolList: symbolList [
 	"read the components for <projectDefinition> to develop the list of dependent projects"
@@ -226,6 +237,27 @@ RwPrjLoadToolV2 >> _loadProjectSetDefinition: projectSetDefinitionToLoad instanc
 	^ loadedProjects
 ]
 
+{ #category : 'private' }
+RwPrjLoadToolV2 >> _loadProjectSetModification: projectSetModification instanceMigrator: instanceMigrator symbolList: symbolList [
+	| loadedProjects |
+	projectSetModification isEmpty
+		ifFalse: [ 
+			| componentsWithDoits |
+			componentsWithDoits := projectSetModification componentsWithDoits.
+			componentsWithDoits do: [ :component | component executePreloadDoit ].
+			Rowan image
+				applyModification_V2: projectSetModification
+				instanceMigrator: instanceMigrator
+				symbolList: symbolList.
+			componentsWithDoits do: [ :component | component executePostloadDoit ] ].
+	loadedProjects := RwLoadedProjectSet new.
+	projectSetModification elementsModified values
+		do: [ :aProjectSetModification | 
+			loadedProjects
+				addProject: (RwProject newNamed: aProjectSetModification after name) ].
+	^ loadedProjects
+]
+
 { #category : 'load project by name' }
 RwPrjLoadToolV2 >> loadProjectNamed: projectName [
 	| projectSet  res |
@@ -289,6 +321,26 @@ RwPrjLoadToolV2 >> loadProjectSetDefinition: projectSetDefinitionToLoad symbolLi
 		symbolList: symbolList
 ]
 
+{ #category : 'load project set modification' }
+RwPrjLoadToolV2 >> loadProjectSetModification: projectSetModification instanceMigrator: instanceMigrator symbolList: symbolList [
+	"NOTE: when loading a definition into a stone, the loaded things are not marked as non-dirty ... dirty state is relative to the disk image for a
+		project and a definition can have come from anywhere"
+
+	^ self
+		_doProjectSetModificationLoad: projectSetModification
+		instanceMigrator: instanceMigrator
+		symbolList: symbolList
+		processedClassNames: Set new
+]
+
+{ #category : 'load project set modification' }
+RwPrjLoadToolV2 >> loadProjectSetModification: projectSetModification symbolList: symbolList [
+	^ self
+		loadProjectSetModification: projectSetModification
+		instanceMigrator: Rowan platform instanceMigrator
+		symbolList: symbolList
+]
+
 { #category : 'utilities' }
 RwPrjLoadToolV2 >> markProjectSetNotDirty: projectSetDefinition [
 	"This method should only be used when the projectSetDefinitions have been read from disk - mark them not dirty. Not dirty means that the loaded things match the code on disk."