Merge pull request #863 from ApptiveGrid/fix-page-recycling

Fix and finish page recycling

src/Soil-Core-Tests/SoilBTreeTest.class.st

@@ -274,12 +274,12 @@ SoilBTreeTest >> testIndexRewriting [
 	1 to: capacity + 1 do: [ :n | 
 		index at: n put: (n asByteArrayOfSize: 8) ].
 	index flush.
-	self assert: index headerPage lastPageIndex equals: 3.
+	self assert: index headerPage lastPageOffset equals: 3.
 	self assert: (index pageAt: 3) items size equals: 128.
 	self assert: (index pageAt: 1) items size equals: 126.
 	fileSizeBefore := index path size.
 	index newPluggableRewriter rewrite.
-	self assert: index headerPage lastPageIndex equals: 3.
+	self assert: index headerPage lastPageOffset equals: 3.
 	self assert: (index pageAt: 3) items size equals: 128.
 	self assert: (index pageAt: 1) items size equals: 126.
 
@@ -299,7 +299,7 @@ SoilBTreeTest >> testIndexRewritingWithCleaning [
 		index at: n  put: SoilObjectId removed ].
 
 	index newPluggableRewriter cleanRemoved run.
-	self assert: index headerPage lastPageIndex equals: 2.
+	self assert: index headerPage lastPageOffset equals: 2.
 	self assert: (index pageAt: 1) items size equals: 1.
 	self assert: (index at: capacity + 1) asByteArray equals: (capacity + 1 asByteArrayOfSize: 8).
 

src/Soil-Core-Tests/SoilSkipListPageTest.class.st

@@ -125,7 +125,7 @@ SoilSkipListPageTest >> testHeaderPageFull [
 		offset: 1;
 		level: 8;
 		pageSize: 4096;
-		lastPageIndex: 2.
+		lastPageOffset: 2.
 	1 to: 253 do: [ :n |
 		page addItem: (n -> n) ].
 	self assert: page hasRoom equals: false.
@@ -146,7 +146,7 @@ SoilSkipListPageTest >> testHeaderPageFullStreamSize [
 		keySize: 7;
 		valueSize: 8;
 		maxLevel: 8;
-		lastPageIndex: 1;
+		lastPageOffset: 1;
 		offset: 1;
 		level: 6;
 		pageSize: 4096.
@@ -171,7 +171,7 @@ SoilSkipListPageTest >> testHeaderPageSizeOfHeader [
 		offset: 1;
 		level: 8;
 		pageSize: 4096;
-		lastPageIndex: 2.
+		lastPageOffset: 2.
 	bytes := ByteArray streamContents: [ :stream |
 		page writeHeaderOn: stream ].
 	self assert: bytes size equals: page headerSize 
@@ -186,7 +186,7 @@ SoilSkipListPageTest >> testHeaderPageWriteAndRead [
 		maxLevel: 12;
 		offset: 1;
 		level: 8;
-		lastPageIndex: 2.
+		lastPageOffset: 2.
 	self assert: page needsWrite.
 	bytes := ByteArray streamContents: [ :stream |
 		page writeOn: stream ].
@@ -196,7 +196,7 @@ SoilSkipListPageTest >> testHeaderPageWriteAndRead [
 	self assert: readPage keySize equals: 16.
 	self assert: readPage valueSize equals: 8.
 	self assert: readPage maxLevel equals: 12.
-	self assert: readPage lastPageIndex equals: 2.
+	self assert: readPage lastPageOffset equals: 2.
 	self deny: readPage needsWrite 
 ]
 
@@ -210,7 +210,7 @@ SoilSkipListPageTest >> testWriteOldVersion [
 		offset: 1;
 		level: 8;
 		pageSize: 4096;
-		lastPageIndex: 2;
+		lastPageOffset: 2;
 		instVarNamed: #version put: 1;
 		yourself.
 	1 to: 10 do: [ :n |

src/Soil-Core-Tests/SoilSkipListTest.class.st

@@ -265,6 +265,112 @@ SoilSkipListTest >> testFirstArgLargerThenSize [
 	self assert: (index first: 5) size equals: 4
 ]
 
+{ #category : #tests }
+SoilSkipListTest >> testFreePageAdd [
+	| iterator |
+	iterator := index newIterator.
+	1 to: 2500 do: [ :n | 
+		iterator at: n  put: (n asByteArrayOfSize: 8) ].
+	self assert: index headerPage lastPageOffset equals: 10.
+	self assert: index headerPage firstFreePageIndex equals: 0.
+	500 to: 1500 do: [ : n |
+		iterator removeKey: n ].
+	self assert: index headerPage lastPageOffset equals: 10.
+	self assert: index headerPage firstFreePageIndex equals: 0.
+	index cleanUpToVersion: nil.
+	self assert: index headerPage lastPageOffset equals: 10.
+	"the first page that gets empty is reused as free list page"
+	self assert: index headerPage firstFreePageIndex equals: 3.
+	"subsequent removed pages are added to the first page"
+	self assertCollection: (index store pageAt: 3) pageIndexes hasSameElements: #( 4 5 ).
+]
+
+{ #category : #tests }
+SoilSkipListTest >> testFreePageAddNested [
+	| iterator nestedFreeIndexes |
+	index
+		maxLevel: 8; 
+		valueSize: 512.
+	iterator := index newIterator.
+	"create enough pages to test"
+	1 to: 9000 do: [ :n | 
+		iterator at: n  put: (n asByteArrayOfSize: 8) ].
+	self assert: index headerPage lastPageOffset equals: 1286.
+	self assert: index headerPage firstFreePageIndex equals: 0.
+	"now we free more pages than a free page list has capacity"
+	10 to: 8990 do: [ : n |
+		iterator removeKey: n ].
+	index cleanUpToVersion: nil.
+	self assert: index headerPage firstFreePageIndex equals: 3.
+	self assert: (index store pageAt: 3) pageIndexes size equals: 1018.
+	"as there were more than free list capacity there is a next free
+	list page"
+	self assert: (index store pageAt: 3) next equals: 1022.
+	nestedFreeIndexes := ((index store pageAt: 1022) pageIndexes copy) copyWith: 1022.
+	"we fill the index with enough elements to use all free pages including the nested 
+	ones"
+	1 to: 15*1018 do: [ :n | 
+		iterator at: n  put: (n asByteArrayOfSize: 8) ].
+	"there should be no more free page"
+	self assert: index headerPage firstFreePageIndex equals: 0.
+	"and all pageIndexes of the next free page should be included"
+	self assert: ((index store pages collect: #offset) includesAll: nestedFreeIndexes) 
+
+
+]
+
+{ #category : #tests }
+SoilSkipListTest >> testFreePageReuse [
+	| iterator |
+	iterator := index newIterator.
+	1 to: 2500 do: [ :n | 
+		iterator at: n  put: (n asByteArrayOfSize: 8) ].
+	self assert: index headerPage lastPageOffset equals: 10.
+	self assert: index headerPage firstFreePageIndex equals: 0.
+	500 to: 1500 do: [ : n |
+		iterator removeKey: n ].
+	index cleanUpToVersion: nil.
+	self assert: index headerPage lastPageOffset equals: 10.
+	2501 to: 2750 do: [ :n | 
+		iterator at: n  put: (n asByteArrayOfSize: 8) ].
+	index cleanUpToVersion: nil.
+	"first free page is #3 with content #( 4 5 ). Adding 250 entries should remove 
+	one page #4 for reuse"
+	self assertCollection: (index store pageAt: 3) pageIndexes hasSameElements: #( 5 ).
+
+]
+
+{ #category : #tests }
+SoilSkipListTest >> testFreePageReuseAtEndAppend [
+	| iterator |
+	iterator := index newIterator.
+	1 to: 2500 do: [ :n | 
+		iterator at: n  put: (n asByteArrayOfSize: 8) ].
+	self assert: index headerPage lastPageOffset equals: 10.
+	self assert: index headerPage firstFreePageIndex equals: 0.
+	500 to: 1500 do: [ : n |
+		iterator removeKey: n ].
+	index cleanUpToVersion: nil.
+	self assert: index headerPage lastPageOffset equals: 10.
+	self assertCollection: (index store pageAt: 3) pageIndexes hasSameElements: #( 4 5 ).
+	iterator := index newIterator.
+	2501 to: 3500 do: [ :n | 
+		iterator at: n  put: (n asByteArrayOfSize: 8) ].
+	index cleanUpToVersion: nil.
+	"readding the same amount of entries but at the end should reuse all 
+	free pages making first free 0."
+	self assert: index headerPage firstFreePageIndex  equals: 0.
+	"as the new entries did not fit exactly we have one more page at the 
+	end which should have update the header page"
+	self assert: index headerPage lastPageOffset equals: 11.
+	"page #3 was the first free page so it was recycled last. Therefor the 
+	page #3 should point to page #11 which should be the last"
+	self assert: (index store pageAt: 3) right first equals: 11.
+	self assert: (index store pageAt: 11) right first equals: 0.
+
+
+]
+
 { #category : #tests }
 SoilSkipListTest >> testIndexRewriting [
 
@@ -275,7 +381,7 @@ SoilSkipListTest >> testIndexRewriting [
 	index flush.
 	fileSizeBefore := index path size.
 	index newPluggableRewriter rewrite.
-	self assert: index headerPage lastPageIndex equals: 2.
+	self assert: index headerPage lastPageOffset equals: 2.
 	self assert: (index pageAt: 2) items size equals: 1.
 	"self assert: index path size equals: fileSizeBefore"
 ]
@@ -292,7 +398,7 @@ SoilSkipListTest >> testIndexRewritingWithCleaning [
 		index at: n  put: SoilObjectId removed ].
 
 	index newPluggableRewriter cleanRemoved rewrite.
-	self assert: index headerPage lastPageIndex equals: 1.
+	self assert: index headerPage lastPageOffset equals: 1.
 	self assert: (index pageAt: 1) items size equals: 1.
 	self assert: (index at: capacity + 1) equals: (capacity + 1 asByteArrayOfSize: 8).
 

src/Soil-Core/SoilBTreeHeaderPage.class.st

@@ -9,9 +9,9 @@ Class {
 	#name : #SoilBTreeHeaderPage,
 	#superclass : #SoilAbstractBTreeDataPage,
 	#instVars : [
-		'lastPageIndex',
 		'firstFreePageIndex',
-		'size'
+		'size',
+		'lastPageOffset'
 	],
 	#category : #'Soil-Core-Index-BTree'
 }
@@ -68,13 +68,13 @@ SoilBTreeHeaderPage >> isHeaderPage [
 ]
 
 { #category : #accessing }
-SoilBTreeHeaderPage >> lastPageIndex [
-	^ lastPageIndex
+SoilBTreeHeaderPage >> lastPageOffset [
+	^ lastPageOffset
 ]
 
 { #category : #accessing }
-SoilBTreeHeaderPage >> lastPageIndex: anObject [
-	lastPageIndex := anObject.
+SoilBTreeHeaderPage >> lastPageOffset: anObject [
+	lastPageOffset := anObject.
 	needWrite := true
 ]
 
@@ -85,9 +85,9 @@ SoilBTreeHeaderPage >> latestVersion [
 
 { #category : #accessing }
 SoilBTreeHeaderPage >> nextPageOffset [
-	lastPageIndex := lastPageIndex + 1.
+	lastPageOffset := lastPageOffset + 1.
 	needWrite := true.
-	^ lastPageIndex 
+	^ lastPageOffset 
 ]
 
 { #category : #printing }
@@ -100,7 +100,7 @@ SoilBTreeHeaderPage >> readFrom: aStream [
 	super readFrom: aStream.
 	keySize := (aStream next: 2) asInteger.
 	valueSize := (aStream next: 2) asInteger.
-	lastPageIndex :=(aStream next: self pointerSize) asInteger.
+	lastPageOffset :=(aStream next: self pointerSize) asInteger.
 	firstFreePageIndex :=(aStream next: 4) asInteger.
 	size := (aStream next: 6) asInteger.
 	self readItemsFrom: aStream
@@ -122,7 +122,7 @@ SoilBTreeHeaderPage >> writeHeaderOn: aStream [
 	aStream
 		nextPutAll: (keySize asByteArrayOfSize: 2);
 		nextPutAll: (valueSize asByteArrayOfSize: 2);
-		nextPutAll: (lastPageIndex asByteArrayOfSize: self pointerSize);
+		nextPutAll: (lastPageOffset asByteArrayOfSize: self pointerSize);
 		nextPutAll: (firstFreePageIndex asByteArrayOfSize: 4);
 		nextPutAll: (self size asByteArrayOfSize: 6).
 ]

src/Soil-Core/SoilBasicBTree.class.st

@@ -89,7 +89,7 @@ SoilBasicBTree >> keySize: anInteger [
 SoilBasicBTree >> newHeaderPage [
 	^ SoilBTreeHeaderPage new
 		offset: 1;
-		lastPageIndex: 2;
+		lastPageOffset: 2;
 		firstFreePageIndex: 0;
 		pageSize: self pageSize
 ]
@@ -135,7 +135,7 @@ SoilBasicBTree >> nextFreePage [
 	^ firstPage hasFreePages 
 		ifTrue: [ 
 			freePage := store pageAt: firstPage removeFirstIndex.
-			self addDirtyPage: freePage.
+			self addDirtyPage: firstPage.
 			freePage  ] 
 		ifFalse: [ 
 			self headerPage firstFreePageIndex: 0.
@@ -154,6 +154,11 @@ SoilBasicBTree >> pageClass [
 	^ SoilBTreeDataPage
 ]
 
+{ #category : #removing }
+SoilBasicBTree >> removeDirtyPage: aPage [ 
+	dirtyPages remove: aPage
+]
+
 { #category : #removing }
 SoilBasicBTree >> removeKey: key ifAbsent: aBlock [
 

src/Soil-Core/SoilBasicSkipList.class.st

@@ -19,7 +19,16 @@ SoilBasicSkipList class >> isAbstract [
 
 { #category : #adding }
 SoilBasicSkipList >> addDirtyPage: aPage [ 
-	dirtyPages add: aPage
+	dirtyPages at: aPage offset put: aPage
+]
+
+{ #category : #'as yet unclassified' }
+SoilBasicSkipList >> cleanUpToVersion: aNumberOrNil [ 
+		SoilIndexCleaner new 
+			index: self;
+			readVersion: aNumberOrNil;
+			clean.
+
 ]
 
 { #category : #testing }
@@ -30,7 +39,7 @@ SoilBasicSkipList >> hasHeaderPage [
 { #category : #initialization }
 SoilBasicSkipList >> initialize [ 
 	super initialize.
-	dirtyPages := Set new
+	dirtyPages := OrderedDictionary new
 ]
 
 { #category : #accessing }

src/Soil-Core/SoilIndex.class.st

@@ -286,7 +286,7 @@ SoilIndex >> recyclePage: aPage [
 
 { #category : #removing }
 SoilIndex >> removeDirtyPage: aPage [ 
-	dirtyPages remove: aPage
+	dirtyPages removeKey: aPage offset
 ]
 
 { #category : #removing }

src/Soil-Core/SoilIndexCleaner.class.st

@@ -22,7 +22,7 @@ SoilIndexCleaner >> clean [
 	that need to be written"
 	self dirtyPages do: [ :page |
 		"if no transaction needs old state and the page needs cleaning"
-		((self canDiscardOldState: page) and: [ page needsCleanup ]) ifTrue: [  
+		(page needsCleanup and: [ self canDiscardOldState: page ]) ifTrue: [  
 			self cleanPage: page ] ].
 	"Now we have a final set if dirty pages"
 	index dirtyPages copy do: [ :page | 

src/Soil-Core/SoilIndexConsistencyVisitor.class.st

@@ -13,7 +13,7 @@ SoilIndexConsistencyVisitor >> check: aSoil [
 { #category : #visiting }
 SoilIndexConsistencyVisitor >> visitPagedFileIndexStore: aSoilPagedFileIndexStore [ 
 	| numberOfPages |
-	numberOfPages := aSoilPagedFileIndexStore headerPage lastPageIndex.
+	numberOfPages := aSoilPagedFileIndexStore headerPage lastPageOffset.
 	1 to: numberOfPages do: [ :pageIndex | | page |
 		page := aSoilPagedFileIndexStore pageAt: pageIndex.
 		(page right allSatisfy: #isZero) ifTrue: [ 

src/Soil-Core/SoilInstanceVisitor.class.st

@@ -42,7 +42,7 @@ SoilInstanceVisitor >> visitObjectId: aSoilObjectId [
 { #category : #visiting }
 SoilInstanceVisitor >> visitPagedFileIndexStore: aSoilPagedFileIndexStore [ 
 	| numberOfPages |
-	numberOfPages := aSoilPagedFileIndexStore headerPage lastPageIndex.
+	numberOfPages := aSoilPagedFileIndexStore headerPage lastPageOffset.
 	1 to: numberOfPages do: [ :pageIndex | | page |
 		page := aSoilPagedFileIndexStore pageAt: pageIndex.
 		page items do: [ :item |

src/Soil-Core/SoilPagedIndexStore.class.st

@@ -69,8 +69,8 @@ SoilPagedIndexStore >> isOpen [
 ]
 
 { #category : #accessing }
-SoilPagedIndexStore >> lastPageIndex [
-	^ self headerPage lastPageIndex
+SoilPagedIndexStore >> lastPageOffset [
+	^ self headerPage lastPageOffset
 ]
 
 { #category : #'instance creation' }