Merge pull request #866 from ApptiveGrid/fix-page-allocation

fixed allocation of pages
ApptiveGrid · Nov 21, 2024 · cca4147 · cca4147
2 parents 304057d + a2480ed
commit cca4147
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 57 deletions.
diff --git a/src/Soil-Core-Tests/SoilSkipListTest.class.st b/src/Soil-Core-Tests/SoilSkipListTest.class.st
@@ -285,6 +285,23 @@ SoilSkipListTest >> testFreePageAdd [
 	self assertCollection: (index store pageAt: 3) pageIndexes hasSameElements: #( 4 5 ).
 ]
 
+{ #category : #tests }
+SoilSkipListTest >> testFreePageAddAndDelete [
+	| iterator |
+	iterator := index newIterator.
+	1 to: 280 do: [ :n | 
+		iterator at: n  put: (n asByteArrayOfSize: 8) ].
+	self assert: index headerPage lastPageOffset equals: 2.
+	self assert: index headerPage firstFreePageIndex equals: 0.
+	1 to: 2000 do: [ : n |
+		iterator at: 280+n put: ((200+n) asByteArrayOfSize: 8).
+		iterator removeKey: n.
+		index cleanUpToVersion: nil. ].
+
+	self assert: index headerPage lastPageOffset equals: 4.
+	self assert: (index store pageAt: 4) pageIndexes isEmpty
+]
+
 { #category : #tests }
 SoilSkipListTest >> testFreePageAddNested [
 	| iterator nestedFreeIndexes |

diff --git a/src/Soil-Core/SoilBasicSkipList.class.st b/src/Soil-Core/SoilBasicSkipList.class.st
@@ -22,6 +22,16 @@ SoilBasicSkipList >> addDirtyPage: aPage [
 	dirtyPages at: aPage offset put: aPage
 ]
 
+{ #category : #'instance creation' }
+SoilBasicSkipList >> allocatePage [
+	| page |
+	page := self newPage 
+		offset: (self nextFreePageIndex
+			ifNil: [ self nextPageOffset ]).
+	self addDirtyPage: page.
+	^ page
+]
+
 { #category : #'as yet unclassified' }
 SoilBasicSkipList >> cleanUpToVersion: aNumberOrNil [ 
 		SoilIndexCleaner new 
@@ -59,6 +69,49 @@ SoilBasicSkipList >> newIterator [
 	^ SoilSkipListIterator on: self 
 ]
 
+{ #category : #'instance creation' }
+SoilBasicSkipList >> newPage [
+	^ self pageClass new 
+		initializeLevel: self maxLevel;
+		keySize: self keySize;
+		valueSize: self valueSize;
+		pageSize: self pageSize;
+		yourself
+]
+
+{ #category : #private }
+SoilBasicSkipList >> nextFreePageIndex [ 
+	| firstFreePage freePageIndex |
+	firstFreePage := self firstFreePage ifNil: [ 
+		^ nil ].
+	^ firstFreePage hasFreePages 
+		ifTrue: [ 
+			freePageIndex := firstFreePage removeFirstIndex.
+			self addDirtyPage: firstFreePage.
+			freePageIndex  ] 
+		ifFalse: [ 
+			"if firstFreePage has a next pointer we set that as 
+			new first free page. Next will return 0 if there is no
+			next page"
+			self headerPage firstFreePageIndex: firstFreePage next.
+			self addDirtyPage: self headerPage.
+			firstFreePage offset ]
+
+]
+
+{ #category : #'instance creation' }
+SoilBasicSkipList >> nextPageOffset [
+	| offset |
+	offset := store nextPageOffset.
+	self addDirtyPage: store headerPage.
+	^ offset 
+]
+
+{ #category : #'instance creation' }
+SoilBasicSkipList >> pageClass [
+	^ SoilSkipListDataPage
+]
+
 { #category : #removing }
 SoilBasicSkipList >> removePage: aPage [
 	| iterator previousPage seen page |

diff --git a/src/Soil-Core/SoilCopyOnWriteSkipList.class.st b/src/Soil-Core/SoilCopyOnWriteSkipList.class.st
@@ -12,21 +12,11 @@ SoilCopyOnWriteSkipList >> addDirtyPage: aPage [
 
 ]
 
-{ #category : #'instance creation' }
-SoilCopyOnWriteSkipList >> allocatePage [
-	^ wrapped allocatePage 
-]
-
 { #category : #testing }
 SoilCopyOnWriteSkipList >> isRegistered [
 	^ wrapped isRegistered
 ]
 
-{ #category : #'instance creation' }
-SoilCopyOnWriteSkipList >> newPage [ 
-	^ wrapped allocatePage
-]
-
 { #category : #accessing }
 SoilCopyOnWriteSkipList >> persistentIndex [
 	^ wrapped persistentIndex 

diff --git a/src/Soil-Core/SoilIndexItemsPage.class.st b/src/Soil-Core/SoilIndexItemsPage.class.st
@@ -57,7 +57,8 @@ SoilIndexItemsPage >> canBeRemoved [
 
 { #category : #running }
 SoilIndexItemsPage >> cleanRemoved [
-	items removeAllSuchThat: [ :each | each value isRemoved ]
+	items removeAllSuchThat: [ :each | each value isRemoved ].
+	needWrite := true
 ]
 
 { #category : #accessing }
@@ -243,7 +244,6 @@ SoilIndexItemsPage >> lastTransaction: anInteger [
 
 { #category : #testing }
 SoilIndexItemsPage >> needsCleanup [ 
-	items size isZero ifTrue: [ ^ true ].
 	^ items size > self presentItemCount 
 ]
 

diff --git a/src/Soil-Core/SoilSkipList.class.st b/src/Soil-Core/SoilSkipList.class.st
@@ -14,16 +14,6 @@ SoilSkipList >> acceptSoil: aSoilVisitor [
 	^ aSoilVisitor visitSkipList: self
 ]
 
-{ #category : #'instance creation' }
-SoilSkipList >> allocatePage [
-	| page |
-	page := self newPage 
-		offset: (self nextFreePageIndex
-			ifNil: [ store nextPageOffset ]).
-	self addDirtyPage: page.
-	^ page
-]
-
 { #category : #converting }
 SoilSkipList >> asCopyOnWrite [
 	^ SoilCopyOnWriteSkipList new
@@ -97,53 +87,18 @@ SoilSkipList >> newHeaderPage [
 		pageSize: self pageSize
 ]
 
-{ #category : #'instance creation' }
-SoilSkipList >> newPage [
-	^ self pageClass new 
-		initializeLevel: self maxLevel;
-		keySize: self keySize;
-		valueSize: self valueSize;
-		pageSize: self pageSize;
-		yourself
-]
-
 { #category : #reindexing }
 SoilSkipList >> newPluggableRewriter [
 	^ SoilPluggableIndexRewriter new 
 		index: self
 ]
 
-{ #category : #private }
-SoilSkipList >> nextFreePageIndex [ 
-	| firstFreePage freePageIndex |
-	firstFreePage := self firstFreePage ifNil: [ 
-		^ nil ].
-	^ firstFreePage hasFreePages 
-		ifTrue: [ 
-			freePageIndex := firstFreePage removeFirstIndex.
-			self addDirtyPage: firstFreePage.
-			freePageIndex  ] 
-		ifFalse: [ 
-			"if firstFreePage has a next pointer we set that as 
-			new first free page. Next will return 0 if there is no
-			next page"
-			self headerPage firstFreePageIndex: firstFreePage next.
-			self addDirtyPage: self headerPage.
-			firstFreePage offset ]
-
-]
-
 { #category : #'opening/closing' }
 SoilSkipList >> open [
 	self isOpen ifTrue: [ self error: 'Index already open' ].
 	self store open
 ]
 
-{ #category : #'instance creation' }
-SoilSkipList >> pageClass [
-	^ SoilSkipListDataPage
-]
-
 { #category : #accessing }
 SoilSkipList >> path [