diff --git a/MODERNIZE.md b/MODERNIZE.md index 8bca8e4..f910a02 100644 --- a/MODERNIZE.md +++ b/MODERNIZE.md @@ -1,16 +1,26 @@ # Package Modernization Instructions ## Goal -Transform legacy packages to use generic MasterList pattern with simplified database operations. +Transform legacy packages to use focused, performance-optimized bespoke master lists with simplified database operations. ## Steps -### 1. Implement Generic MasterList Base -Create `internal/common/master_list.go` with generic collection management: +### 1. Create Bespoke MasterList Implementation +Replace generic implementations with specialized, high-performance master lists: ```go -type MasterList[K comparable, V Identifiable[K]] struct { - items map[K]V +// Create domain-specific master list optimized for the use case +type MasterList struct { + // Core storage + items map[K]*Type // ID -> Type mutex sync.RWMutex + + // Specialized indices for O(1) lookups + byCategory map[string][]*Type // Category -> items + byProperty map[string][]*Type // Property -> items + + // Cached metadata with invalidation + categories []string + metaStale bool } ``` @@ -19,16 +29,18 @@ type MasterList[K comparable, V Identifiable[K]] struct { **Remove:** - Legacy wrapper functions (LoadAll, SaveAll, etc.) - Duplicate database files (database.go, database_legacy.go) +- Generic MasterList dependencies - README.md (use doc.go instead) - Separate "active_record.go" files **Consolidate into:** - `{type}.go` - Main type with embedded database operations - `types.go` - Supporting types only (no main type) -- `master.go` - MasterList using generic base +- `master.go` - Bespoke MasterList optimized for domain - `player.go` - Player-specific logic (if applicable) - `doc.go` - Primary documentation - `{type}_test.go` - Focused tests +- `benchmark_test.go` - Comprehensive performance tests ### 3. Refactor Main Type @@ -60,61 +72,114 @@ func (a *Achievement) Delete() error func (a *Achievement) Reload() error ``` -### 4. Update MasterList +### 4. Create Bespoke MasterList -Replace manual implementation with generic base: +Replace generic implementations with specialized, optimized master lists: ```go -// Before: Manual thread-safety +// Before: Generic base with limited optimization type MasterList struct { - items map[uint32]*Achievement - mutex sync.RWMutex + *common.MasterList[uint32, *Achievement] +} + +// After: Bespoke implementation optimized for domain +type MasterList struct { + // Core storage + achievements map[uint32]*Achievement + mutex sync.RWMutex + + // Domain-specific indices for O(1) lookups + byCategory map[string][]*Achievement + byExpansion map[string][]*Achievement + + // Cached metadata with lazy loading + categories []string + expansions []string + metaStale bool } func (m *MasterList) AddAchievement(a *Achievement) bool { m.mutex.Lock() defer m.mutex.Unlock() - // manual implementation -} - -// After: Generic base -type MasterList struct { - *common.MasterList[uint32, *Achievement] -} - -func NewMasterList() *MasterList { - return &MasterList{ - MasterList: common.NewMasterList[uint32, *Achievement](), + + // Check existence + if _, exists := m.achievements[a.AchievementID]; exists { + return false } + + // Add to core storage + m.achievements[a.AchievementID] = a + + // Update specialized indices + m.byCategory[a.Category] = append(m.byCategory[a.Category], a) + m.byExpansion[a.Expansion] = append(m.byExpansion[a.Expansion], a) + + // Invalidate metadata cache + m.metaStale = true + return true } -func (m *MasterList) AddAchievement(a *Achievement) bool { - return m.MasterList.Add(a) +// O(1) category lookup +func (m *MasterList) GetByCategory(category string) []*Achievement { + m.mutex.RLock() + defer m.mutex.RUnlock() + return m.byCategory[category] } ``` -### 5. Implement Identifiable Interface +### 5. Optimize for Domain Use Cases -Ensure main type implements `GetID()`: +Focus on the specific operations your domain needs: ```go +// Implement GetID() for consistency func (a *Achievement) GetID() uint32 { - return a.AchievementID // or appropriate ID field + return a.AchievementID +} + +// Add domain-specific optimized methods +func (m *MasterList) GetByCategoryAndExpansion(category, expansion string) []*Achievement { + // Use set intersection for efficient combined queries + categoryItems := m.byCategory[category] + expansionItems := m.byExpansion[expansion] + + // Use smaller set for iteration efficiency + if len(categoryItems) > len(expansionItems) { + categoryItems, expansionItems = expansionItems, categoryItems + } + + // Set intersection using map lookup + expansionSet := make(map[*Achievement]struct{}, len(expansionItems)) + for _, item := range expansionItems { + expansionSet[item] = struct{}{} + } + + var result []*Achievement + for _, item := range categoryItems { + if _, exists := expansionSet[item]; exists { + result = append(result, item) + } + } + return result } ``` ### 6. Simplify API **Remove:** +- Generic MasterList dependencies - Legacy type variants (Achievement vs AchievementRecord) - Conversion methods (ToLegacy, FromLegacy) - Duplicate CRUD operations - Complex wrapper functions +- Slow O(n) filter operations **Keep:** - Single type definition - Direct database methods on type -- Domain-specific extensions only +- Domain-specific optimized operations +- O(1) indexed lookups where possible +- Lazy caching for expensive operations ### 7. Update Documentation @@ -132,10 +197,19 @@ Create concise `doc.go`: // loaded, _ := achievements.Load(db, 1001) // loaded.Delete() // -// Master List: +// Bespoke Master List (optimized for performance): // // masterList := achievements.NewMasterList() -// masterList.Add(achievement) +// masterList.AddAchievement(achievement) +// +// // O(1) category lookup +// combatAchievements := masterList.GetByCategory("Combat") +// +// // O(1) expansion lookup +// classicAchievements := masterList.GetByExpansion("Classic") +// +// // Optimized set intersection +// combined := masterList.GetByCategoryAndExpansion("Combat", "Classic") package achievements ``` @@ -151,32 +225,44 @@ Create focused tests: 1. **Single Source of Truth**: One type definition, not multiple variants 2. **Embedded Operations**: Database methods on the type itself -3. **Generic Base**: Use common.MasterList for thread-safety -4. **No Legacy Baggage**: Remove all "Legacy" types and converters -5. **Documentation in Code**: Use doc.go, not README.md +3. **Bespoke Master Lists**: Domain-specific optimized implementations +4. **Performance First**: O(1) lookups, lazy caching, efficient algorithms +5. **Thread Safety**: Proper RWMutex usage with minimal lock contention +6. **No Legacy Baggage**: Remove all "Legacy" types and converters +7. **Documentation in Code**: Use doc.go, not README.md +8. **Comprehensive Benchmarking**: Measure and optimize all operations ## Migration Checklist -- [ ] Create/verify generic MasterList in common package - [ ] Identify main type and supporting types - [ ] Consolidate database operations into main type - [ ] Add db field and methods to main type -- [ ] Replace manual MasterList with generic base -- [ ] Implement GetID() for Identifiable interface +- [ ] **Create bespoke MasterList optimized for domain** +- [ ] **Add specialized indices for O(1) lookups** +- [ ] **Implement lazy caching for expensive operations** +- [ ] **Add set intersection algorithms for combined queries** +- [ ] Implement GetID() for consistency +- [ ] Remove all generic MasterList dependencies - [ ] Remove all legacy types and converters -- [ ] Update doc.go with concise examples +- [ ] Update doc.go with performance-focused examples - [ ] Simplify tests to cover core functionality +- [ ] **Add concurrency tests for thread safety** - [ ] **Create comprehensive benchmarks (benchmark_test.go)** -- [ ] **Verify performance meets targets** +- [ ] **Verify performance meets targets (sub-microsecond operations)** +- [ ] Run `go test -race` to verify thread safety - [ ] Run `go fmt`, `go test`, and `go test -bench=.` ## Expected Results - **80% less code** in most packages - **Single type** instead of multiple variants -- **Thread-safe** operations via generic base +- **Thread-safe** operations with optimized locking +- **O(1) performance** for common lookup operations +- **Sub-microsecond** response times for most operations +- **Specialized algorithms** for domain-specific queries - **Consistent API** across all packages -- **Better maintainability** with less duplication +- **Better maintainability** with focused, understandable code +- **Performance predictability** through comprehensive benchmarking ## Benchmarking @@ -242,12 +328,15 @@ func (p *mockPlayer) GetLocation() int32 { return p.location } ### Performance Expectations -Target performance after modernization: +Target performance after bespoke modernization: - **Creation operations**: <100ns per operation -- **Lookup operations**: <50ns per operation -- **Collection operations**: O(1) for gets, O(N) for filters -- **Memory allocations**: Minimize in hot paths -- **Concurrent access**: Linear scaling with cores +- **ID lookup operations**: <50ns per operation (O(1) map access) +- **Indexed lookup operations**: <100ns per operation (O(1) specialized indices) +- **Combined queries**: <5µs per operation (optimized set intersection) +- **Cached metadata**: <100ns per operation (lazy loading) +- **Memory allocations**: Zero allocations for read operations +- **Concurrent access**: Linear scaling with cores, minimal lock contention +- **Specialized algorithms**: Domain-optimized performance characteristics ### Running Benchmarks @@ -268,15 +357,23 @@ go test -bench=BenchmarkCoreAlgorithm -cpuprofile=cpu.prof ./internal/package_na ## Example Commands ```bash -# Remove legacy files +# Remove legacy files and generic dependencies rm README.md database_legacy.go active_record.go +# Remove generic MasterList imports +grep -r "eq2emu/internal/common" . --include="*.go" | cut -d: -f1 | sort -u | xargs sed -i '/eq2emu\/internal\/common/d' + # Rename if needed mv active_record.go achievement.go # Test the changes go fmt ./... go test ./... +go test -race ./... # Verify thread safety go test -bench=. ./... go build ./... + +# Performance verification +go test -bench=BenchmarkMasterListOperations -benchtime=5s ./internal/package_name +go test -bench=. -benchmem ./internal/package_name ``` \ No newline at end of file diff --git a/internal/alt_advancement/alt_advancement_test.go b/internal/alt_advancement/alt_advancement_test.go index 1f4bc2b..cb01408 100644 --- a/internal/alt_advancement/alt_advancement_test.go +++ b/internal/alt_advancement/alt_advancement_test.go @@ -1,6 +1,7 @@ package alt_advancement import ( + "sync" "testing" "eq2emu/internal/database" @@ -61,8 +62,8 @@ func TestSimpleAltAdvancement(t *testing.T) { } } -// TestMasterListWithGeneric tests the master list with generic base -func TestMasterListWithGeneric(t *testing.T) { +// TestMasterList tests the bespoke master list implementation +func TestMasterList(t *testing.T) { masterList := NewMasterList() if masterList == nil { @@ -73,25 +74,64 @@ func TestMasterListWithGeneric(t *testing.T) { t.Errorf("Expected size 0, got %d", masterList.Size()) } - // Create an AA (need database for new pattern) - db, _ := database.NewSQLite("file::memory:?mode=memory&cache=shared") + // Create test database + db, err := database.NewSQLite("file::memory:?mode=memory&cache=shared") + if err != nil { + t.Fatalf("Failed to create test database: %v", err) + } defer db.Close() - aa := New(db) - aa.SpellID = 1001 - aa.NodeID = 1001 - aa.Name = "Dragon's Strength" - aa.Group = AA_CLASS - aa.RankCost = 1 - aa.MaxRank = 5 + // Create AAs for testing + aa1 := New(db) + aa1.SpellID = 1001 + aa1.NodeID = 1001 + aa1.Name = "Dragon's Strength" + aa1.Group = AA_CLASS + aa1.ClassReq = 1 // Fighter + aa1.MinLevel = 10 + aa1.RankCost = 1 + aa1.MaxRank = 5 + + aa2 := New(db) + aa2.SpellID = 1002 + aa2.NodeID = 1002 + aa2.Name = "Spell Mastery" + aa2.Group = AA_SUBCLASS + aa2.ClassReq = 2 // Mage + aa2.MinLevel = 15 + aa2.RankCost = 2 + aa2.MaxRank = 3 + + aa3 := New(db) + aa3.SpellID = 1003 + aa3.NodeID = 1003 + aa3.Name = "Universal Skill" + aa3.Group = AA_CLASS + aa3.ClassReq = 0 // Universal (no class requirement) + aa3.MinLevel = 5 + aa3.RankCost = 1 + aa3.MaxRank = 10 // Test adding - if !masterList.AddAltAdvancement(aa) { - t.Error("Should successfully add alternate advancement") + if !masterList.AddAltAdvancement(aa1) { + t.Error("Should successfully add aa1") } - if masterList.Size() != 1 { - t.Errorf("Expected size 1, got %d", masterList.Size()) + if !masterList.AddAltAdvancement(aa2) { + t.Error("Should successfully add aa2") + } + + if !masterList.AddAltAdvancement(aa3) { + t.Error("Should successfully add aa3") + } + + if masterList.Size() != 3 { + t.Errorf("Expected size 3, got %d", masterList.Size()) + } + + // Test duplicate add (should fail) + if masterList.AddAltAdvancement(aa1) { + t.Error("Should not add duplicate alternate advancement") } // Test retrieving @@ -104,10 +144,112 @@ func TestMasterListWithGeneric(t *testing.T) { t.Errorf("Expected name 'Dragon's Strength', got '%s'", retrieved.Name) } - // Test filtering + // Test group filtering classAAs := masterList.GetAltAdvancementsByGroup(AA_CLASS) - if len(classAAs) != 1 { - t.Errorf("Expected 1 AA in Class group, got %d", len(classAAs)) + if len(classAAs) != 2 { + t.Errorf("Expected 2 AAs in Class group, got %d", len(classAAs)) + } + + subclassAAs := masterList.GetAltAdvancementsByGroup(AA_SUBCLASS) + if len(subclassAAs) != 1 { + t.Errorf("Expected 1 AA in Subclass group, got %d", len(subclassAAs)) + } + + // Test class filtering (includes universal AAs) + fighterAAs := masterList.GetAltAdvancementsByClass(1) + if len(fighterAAs) != 2 { + t.Errorf("Expected 2 AAs for Fighter (1 specific + 1 universal), got %d", len(fighterAAs)) + } + + mageAAs := masterList.GetAltAdvancementsByClass(2) + if len(mageAAs) != 2 { + t.Errorf("Expected 2 AAs for Mage (1 specific + 1 universal), got %d", len(mageAAs)) + } + + // Test level filtering + level10AAs := masterList.GetAltAdvancementsByLevel(10) + if len(level10AAs) != 2 { + t.Errorf("Expected 2 AAs available at level 10 (levels 5 and 10), got %d", len(level10AAs)) + } + + level20AAs := masterList.GetAltAdvancementsByLevel(20) + if len(level20AAs) != 3 { + t.Errorf("Expected 3 AAs available at level 20 (all), got %d", len(level20AAs)) + } + + // Test combined filtering + combined := masterList.GetAltAdvancementsByGroupAndClass(AA_CLASS, 1) + if len(combined) != 2 { + t.Errorf("Expected 2 AAs matching Class+Fighter, got %d", len(combined)) + } + + // Test metadata caching + groups := masterList.GetGroups() + if len(groups) != 2 { + t.Errorf("Expected 2 unique groups, got %d", len(groups)) + } + + classes := masterList.GetClasses() + if len(classes) != 2 { + t.Errorf("Expected 2 unique classes (1,2), got %d", len(classes)) + } + + // Test clone + clone := masterList.GetAltAdvancementClone(1001) + if clone == nil { + t.Error("Should return cloned alternate advancement") + } + + if clone.Name != "Dragon's Strength" { + t.Errorf("Expected cloned name 'Dragon's Strength', got '%s'", clone.Name) + } + + // Test GetAllAltAdvancements + allAAs := masterList.GetAllAltAdvancements() + if len(allAAs) != 3 { + t.Errorf("Expected 3 AAs in GetAll, got %d", len(allAAs)) + } + + // Test update + updatedAA := New(db) + updatedAA.SpellID = 1001 + updatedAA.NodeID = 1001 + updatedAA.Name = "Updated Strength" + updatedAA.Group = AA_SUBCLASS + updatedAA.ClassReq = 3 + updatedAA.MinLevel = 20 + updatedAA.RankCost = 3 + updatedAA.MaxRank = 7 + + if err := masterList.UpdateAltAdvancement(updatedAA); err != nil { + t.Errorf("Update should succeed: %v", err) + } + + // Verify update worked + retrievedUpdated := masterList.GetAltAdvancement(1001) + if retrievedUpdated.Name != "Updated Strength" { + t.Errorf("Expected updated name 'Updated Strength', got '%s'", retrievedUpdated.Name) + } + + // Verify group index updated + subclassUpdatedAAs := masterList.GetAltAdvancementsByGroup(AA_SUBCLASS) + if len(subclassUpdatedAAs) != 2 { + t.Errorf("Expected 2 AAs in Subclass group after update, got %d", len(subclassUpdatedAAs)) + } + + // Test removal + if !masterList.RemoveAltAdvancement(1001) { + t.Error("Should successfully remove alternate advancement") + } + + if masterList.Size() != 2 { + t.Errorf("Expected size 2 after removal, got %d", masterList.Size()) + } + + // Test clear + masterList.Clear() + if masterList.Size() != 0 { + t.Errorf("Expected size 0 after clear, got %d", masterList.Size()) } } @@ -140,3 +282,72 @@ func TestAltAdvancementValidation(t *testing.T) { t.Error("Invalid AA (missing name) should fail validation") } } + +// TestMasterListConcurrency tests thread safety of the master list +func TestMasterListConcurrency(t *testing.T) { + masterList := NewMasterList() + + // Create test database + db, err := database.NewSQLite("file::memory:?mode=memory&cache=shared") + if err != nil { + t.Fatalf("Failed to create test database: %v", err) + } + defer db.Close() + + const numWorkers = 10 + const aasPerWorker = 100 + var wg sync.WaitGroup + + // Concurrently add alternate advancements + wg.Add(numWorkers) + for i := 0; i < numWorkers; i++ { + go func(workerID int) { + defer wg.Done() + for j := 0; j < aasPerWorker; j++ { + aa := New(db) + aa.NodeID = int32(workerID*aasPerWorker + j + 1) + aa.SpellID = aa.NodeID + aa.Name = "Concurrent Test" + aa.Group = AA_CLASS + aa.ClassReq = int8((workerID % 3) + 1) + aa.MinLevel = int8((j % 20) + 1) + aa.RankCost = 1 + aa.MaxRank = 5 + masterList.AddAltAdvancement(aa) + } + }(i) + } + + // Concurrently read alternate advancements + wg.Add(numWorkers) + for i := 0; i < numWorkers; i++ { + go func() { + defer wg.Done() + for j := 0; j < aasPerWorker; j++ { + // Random reads + _ = masterList.GetAltAdvancement(int32(j + 1)) + _ = masterList.GetAltAdvancementsByGroup(AA_CLASS) + _ = masterList.GetAltAdvancementsByClass(1) + _ = masterList.Size() + } + }() + } + + wg.Wait() + + // Verify final state + expectedSize := numWorkers * aasPerWorker + if masterList.Size() != expectedSize { + t.Errorf("Expected size %d, got %d", expectedSize, masterList.Size()) + } + + groups := masterList.GetGroups() + if len(groups) != 1 || groups[0] != AA_CLASS { + t.Errorf("Expected 1 group 'AA_CLASS', got %v", groups) + } + + classes := masterList.GetClasses() + if len(classes) != 3 { + t.Errorf("Expected 3 classes, got %d", len(classes)) + } +} diff --git a/internal/alt_advancement/benchmark_test.go b/internal/alt_advancement/benchmark_test.go new file mode 100644 index 0000000..ed13262 --- /dev/null +++ b/internal/alt_advancement/benchmark_test.go @@ -0,0 +1,405 @@ +package alt_advancement + +import ( + "fmt" + "math/rand" + "sync" + "testing" + + "eq2emu/internal/database" +) + +// Global shared master list for benchmarks to avoid repeated setup +var ( + sharedAltAdvancementMasterList *MasterList + sharedAltAdvancements []*AltAdvancement + altAdvancementSetupOnce sync.Once +) + +// setupSharedAltAdvancementMasterList creates the shared master list once +func setupSharedAltAdvancementMasterList(b *testing.B) { + altAdvancementSetupOnce.Do(func() { + // Create test database + db, err := database.NewSQLite("file::memory:?mode=memory&cache=shared") + if err != nil { + b.Fatalf("Failed to create test database: %v", err) + } + + sharedAltAdvancementMasterList = NewMasterList() + + // Pre-populate with alternate advancements for realistic testing + const numAltAdvancements = 1000 + sharedAltAdvancements = make([]*AltAdvancement, numAltAdvancements) + + groups := []int8{AA_CLASS, AA_SUBCLASS, AA_SHADOW, AA_HEROIC, AA_TRADESKILL, AA_PRESTIGE} + classes := []int8{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10} // 0 = universal, 1-10 = specific classes + + for i := range numAltAdvancements { + sharedAltAdvancements[i] = New(db) + sharedAltAdvancements[i].NodeID = int32(i + 1) + sharedAltAdvancements[i].SpellID = int32(i + 1) + sharedAltAdvancements[i].Name = fmt.Sprintf("Alt Advancement %d", i+1) + sharedAltAdvancements[i].Group = groups[i%len(groups)] + sharedAltAdvancements[i].ClassReq = classes[i%len(classes)] + sharedAltAdvancements[i].MinLevel = int8(rand.Intn(50) + 1) + sharedAltAdvancements[i].RankCost = int8(rand.Intn(5) + 1) + sharedAltAdvancements[i].MaxRank = int8(rand.Intn(10) + 1) + sharedAltAdvancements[i].Col = int8(rand.Intn(11)) + sharedAltAdvancements[i].Row = int8(rand.Intn(16)) + + sharedAltAdvancementMasterList.AddAltAdvancement(sharedAltAdvancements[i]) + } + }) +} + +// createTestAltAdvancement creates an alternate advancement for benchmarking +func createTestAltAdvancement(b *testing.B, id int32) *AltAdvancement { + b.Helper() + + // Use nil database for benchmarking in-memory operations + aa := New(nil) + aa.NodeID = id + aa.SpellID = id + aa.Name = fmt.Sprintf("Benchmark AA %d", id) + aa.Group = []int8{AA_CLASS, AA_SUBCLASS, AA_SHADOW, AA_HEROIC}[id%4] + aa.ClassReq = int8((id % 10) + 1) + aa.MinLevel = int8((id % 50) + 1) + aa.RankCost = int8(rand.Intn(5) + 1) + aa.MaxRank = int8(rand.Intn(10) + 1) + aa.Col = int8(rand.Intn(11)) + aa.Row = int8(rand.Intn(16)) + + return aa +} + +// BenchmarkAltAdvancementCreation measures alternate advancement creation performance +func BenchmarkAltAdvancementCreation(b *testing.B) { + db, err := database.NewSQLite("file::memory:?mode=memory&cache=shared") + if err != nil { + b.Fatalf("Failed to create test database: %v", err) + } + defer db.Close() + + b.ResetTimer() + + b.Run("Sequential", func(b *testing.B) { + for i := 0; i < b.N; i++ { + aa := New(db) + aa.NodeID = int32(i) + aa.SpellID = int32(i) + aa.Name = fmt.Sprintf("AA %d", i) + _ = aa + } + }) + + b.Run("Parallel", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + id := int32(0) + for pb.Next() { + aa := New(db) + aa.NodeID = id + aa.SpellID = id + aa.Name = fmt.Sprintf("AA %d", id) + id++ + _ = aa + } + }) + }) +} + +// BenchmarkAltAdvancementOperations measures individual alternate advancement operations +func BenchmarkAltAdvancementOperations(b *testing.B) { + aa := createTestAltAdvancement(b, 1001) + + b.Run("GetID", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _ = aa.GetID() + } + }) + }) + + b.Run("IsNew", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _ = aa.IsNew() + } + }) + }) + + b.Run("Clone", func(b *testing.B) { + for b.Loop() { + _ = aa.Clone() + } + }) + + b.Run("IsValid", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _ = aa.IsValid() + } + }) + }) +} + +// BenchmarkMasterListOperations measures master list performance +func BenchmarkMasterListOperations(b *testing.B) { + setupSharedAltAdvancementMasterList(b) + ml := sharedAltAdvancementMasterList + + b.Run("GetAltAdvancement", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + id := int32(rand.Intn(1000) + 1) + _ = ml.GetAltAdvancement(id) + } + }) + }) + + b.Run("AddAltAdvancement", func(b *testing.B) { + // Create a separate master list for add operations + addML := NewMasterList() + startID := int32(10000) + // Pre-create AAs to measure just the Add operation + aasToAdd := make([]*AltAdvancement, b.N) + for i := 0; i < b.N; i++ { + aasToAdd[i] = createTestAltAdvancement(b, startID+int32(i)) + } + b.ResetTimer() + for i := 0; i < b.N; i++ { + addML.AddAltAdvancement(aasToAdd[i]) + } + }) + + b.Run("GetAltAdvancementsByGroup", func(b *testing.B) { + groups := []int8{AA_CLASS, AA_SUBCLASS, AA_SHADOW, AA_HEROIC, AA_TRADESKILL, AA_PRESTIGE} + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + group := groups[rand.Intn(len(groups))] + _ = ml.GetAltAdvancementsByGroup(group) + } + }) + }) + + b.Run("GetAltAdvancementsByClass", func(b *testing.B) { + classes := []int8{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10} + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + class := classes[rand.Intn(len(classes))] + _ = ml.GetAltAdvancementsByClass(class) + } + }) + }) + + b.Run("GetAltAdvancementsByLevel", func(b *testing.B) { + for b.Loop() { + level := int8(rand.Intn(50) + 1) + _ = ml.GetAltAdvancementsByLevel(level) + } + }) + + b.Run("GetAltAdvancementsByGroupAndClass", func(b *testing.B) { + groups := []int8{AA_CLASS, AA_SUBCLASS, AA_SHADOW, AA_HEROIC} + classes := []int8{1, 2, 3, 4, 5} + for b.Loop() { + group := groups[rand.Intn(len(groups))] + class := classes[rand.Intn(len(classes))] + _ = ml.GetAltAdvancementsByGroupAndClass(group, class) + } + }) + + b.Run("GetGroups", func(b *testing.B) { + for b.Loop() { + _ = ml.GetGroups() + } + }) + + b.Run("GetClasses", func(b *testing.B) { + for b.Loop() { + _ = ml.GetClasses() + } + }) + + b.Run("Size", func(b *testing.B) { + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _ = ml.Size() + } + }) + }) +} + +// BenchmarkConcurrentOperations tests mixed workload performance +func BenchmarkConcurrentOperations(b *testing.B) { + setupSharedAltAdvancementMasterList(b) + ml := sharedAltAdvancementMasterList + + b.Run("MixedOperations", func(b *testing.B) { + groups := []int8{AA_CLASS, AA_SUBCLASS, AA_SHADOW, AA_HEROIC, AA_TRADESKILL, AA_PRESTIGE} + classes := []int8{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10} + + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + switch rand.Intn(8) { + case 0: + id := int32(rand.Intn(1000) + 1) + _ = ml.GetAltAdvancement(id) + case 1: + group := groups[rand.Intn(len(groups))] + _ = ml.GetAltAdvancementsByGroup(group) + case 2: + class := classes[rand.Intn(len(classes))] + _ = ml.GetAltAdvancementsByClass(class) + case 3: + level := int8(rand.Intn(50) + 1) + _ = ml.GetAltAdvancementsByLevel(level) + case 4: + group := groups[rand.Intn(len(groups))] + class := classes[rand.Intn(len(classes))] + _ = ml.GetAltAdvancementsByGroupAndClass(group, class) + case 5: + _ = ml.GetGroups() + case 6: + _ = ml.GetClasses() + case 7: + _ = ml.Size() + } + } + }) + }) +} + +// BenchmarkMemoryAllocation measures memory allocation patterns +func BenchmarkMemoryAllocation(b *testing.B) { + db, err := database.NewSQLite("file::memory:?mode=memory&cache=shared") + if err != nil { + b.Fatalf("Failed to create test database: %v", err) + } + defer db.Close() + + b.Run("AltAdvancementAllocation", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + aa := New(db) + aa.NodeID = int32(i) + aa.SpellID = int32(i) + aa.Name = fmt.Sprintf("AA %d", i) + _ = aa + } + }) + + b.Run("MasterListAllocation", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + ml := NewMasterList() + _ = ml + } + }) + + b.Run("AddAltAdvancement_Allocations", func(b *testing.B) { + b.ReportAllocs() + ml := NewMasterList() + for i := 0; i < b.N; i++ { + aa := createTestAltAdvancement(b, int32(i+1)) + ml.AddAltAdvancement(aa) + } + }) + + b.Run("GetAltAdvancementsByGroup_Allocations", func(b *testing.B) { + setupSharedAltAdvancementMasterList(b) + ml := sharedAltAdvancementMasterList + b.ReportAllocs() + b.ResetTimer() + for b.Loop() { + _ = ml.GetAltAdvancementsByGroup(AA_CLASS) + } + }) + + b.Run("GetGroups_Allocations", func(b *testing.B) { + setupSharedAltAdvancementMasterList(b) + ml := sharedAltAdvancementMasterList + b.ReportAllocs() + b.ResetTimer() + for b.Loop() { + _ = ml.GetGroups() + } + }) +} + +// BenchmarkUpdateOperations measures update performance +func BenchmarkUpdateOperations(b *testing.B) { + setupSharedAltAdvancementMasterList(b) + ml := sharedAltAdvancementMasterList + + b.Run("UpdateAltAdvancement", func(b *testing.B) { + // Create AAs to update + updateAAs := make([]*AltAdvancement, b.N) + for i := 0; i < b.N; i++ { + updateAAs[i] = createTestAltAdvancement(b, int32((i%1000)+1)) + updateAAs[i].Name = "Updated Name" + updateAAs[i].Group = AA_SUBCLASS + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = ml.UpdateAltAdvancement(updateAAs[i]) + } + }) + + b.Run("RemoveAltAdvancement", func(b *testing.B) { + // Create a separate master list for removal testing + removeML := NewMasterList() + + // Add AAs to remove + for i := 0; i < b.N; i++ { + aa := createTestAltAdvancement(b, int32(i+1)) + removeML.AddAltAdvancement(aa) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + removeML.RemoveAltAdvancement(int32(i + 1)) + } + }) +} + +// BenchmarkValidation measures validation performance +func BenchmarkValidation(b *testing.B) { + setupSharedAltAdvancementMasterList(b) + ml := sharedAltAdvancementMasterList + + b.Run("ValidateAll", func(b *testing.B) { + for b.Loop() { + _ = ml.ValidateAll() + } + }) + + b.Run("IndividualValidation", func(b *testing.B) { + aa := createTestAltAdvancement(b, 1001) + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + _ = aa.IsValid() + } + }) + }) +} + +// BenchmarkCloneOperations measures cloning performance +func BenchmarkCloneOperations(b *testing.B) { + setupSharedAltAdvancementMasterList(b) + ml := sharedAltAdvancementMasterList + + b.Run("GetAltAdvancementClone", func(b *testing.B) { + for b.Loop() { + id := int32(rand.Intn(1000) + 1) + _ = ml.GetAltAdvancementClone(id) + } + }) + + b.Run("DirectClone", func(b *testing.B) { + aa := createTestAltAdvancement(b, 1001) + for b.Loop() { + _ = aa.Clone() + } + }) +} diff --git a/internal/alt_advancement/master.go b/internal/alt_advancement/master.go index a652e88..cef6066 100644 --- a/internal/alt_advancement/master.go +++ b/internal/alt_advancement/master.go @@ -2,133 +2,376 @@ package alt_advancement import ( "fmt" - - "eq2emu/internal/common" + "sync" ) -// MasterList manages the global list of all alternate advancements +// MasterList is a specialized alternate advancement master list optimized for: +// - Fast ID-based lookups (O(1)) +// - Fast group-based lookups (O(1)) +// - Fast class-based lookups (O(1)) +// - Fast level-based lookups (O(1)) +// - Efficient filtering and prerequisite validation type MasterList struct { - *common.MasterList[int32, *AltAdvancement] + // Core storage + altAdvancements map[int32]*AltAdvancement // NodeID -> AltAdvancement + mutex sync.RWMutex + + // Group indices for O(1) lookups + byGroup map[int8][]*AltAdvancement // Group -> AAs + byClass map[int8][]*AltAdvancement // ClassReq -> AAs + byLevel map[int8][]*AltAdvancement // MinLevel -> AAs + + // Cached metadata + groups []int8 // Unique groups (cached) + classes []int8 // Unique classes (cached) + metaStale bool // Whether metadata cache needs refresh } -// NewMasterList creates a new master alternate advancement list +// NewMasterList creates a new specialized alternate advancement master list func NewMasterList() *MasterList { return &MasterList{ - MasterList: common.NewMasterList[int32, *AltAdvancement](), + altAdvancements: make(map[int32]*AltAdvancement), + byGroup: make(map[int8][]*AltAdvancement), + byClass: make(map[int8][]*AltAdvancement), + byLevel: make(map[int8][]*AltAdvancement), + metaStale: true, } } -// AddAltAdvancement adds an alternate advancement to the master list -// Returns false if AA with same ID already exists +// refreshMetaCache updates the groups and classes cache +func (m *MasterList) refreshMetaCache() { + if !m.metaStale { + return + } + + groupSet := make(map[int8]struct{}) + classSet := make(map[int8]struct{}) + + // Collect unique groups and classes + for _, aa := range m.altAdvancements { + groupSet[aa.Group] = struct{}{} + if aa.ClassReq > 0 { + classSet[aa.ClassReq] = struct{}{} + } + } + + // Clear existing caches and rebuild + m.groups = m.groups[:0] + for group := range groupSet { + m.groups = append(m.groups, group) + } + + m.classes = m.classes[:0] + for class := range classSet { + m.classes = append(m.classes, class) + } + + m.metaStale = false +} + +// AddAltAdvancement adds an alternate advancement with full indexing func (m *MasterList) AddAltAdvancement(aa *AltAdvancement) bool { if aa == nil { return false } - return m.MasterList.Add(aa) + + m.mutex.Lock() + defer m.mutex.Unlock() + + // Check if exists + if _, exists := m.altAdvancements[aa.NodeID]; exists { + return false + } + + // Add to core storage + m.altAdvancements[aa.NodeID] = aa + + // Update group index + m.byGroup[aa.Group] = append(m.byGroup[aa.Group], aa) + + // Update class index + m.byClass[aa.ClassReq] = append(m.byClass[aa.ClassReq], aa) + + // Update level index + m.byLevel[aa.MinLevel] = append(m.byLevel[aa.MinLevel], aa) + + // Invalidate metadata cache + m.metaStale = true + + return true } -// GetAltAdvancement retrieves an alternate advancement by node ID -// Returns nil if not found +// GetAltAdvancement retrieves by node ID (O(1)) func (m *MasterList) GetAltAdvancement(nodeID int32) *AltAdvancement { - return m.MasterList.Get(nodeID) + m.mutex.RLock() + defer m.mutex.RUnlock() + return m.altAdvancements[nodeID] } // GetAltAdvancementClone retrieves a cloned copy of an alternate advancement by node ID -// Returns nil if not found. Safe for modification without affecting master list func (m *MasterList) GetAltAdvancementClone(nodeID int32) *AltAdvancement { - aa := m.MasterList.Get(nodeID) + m.mutex.RLock() + defer m.mutex.RUnlock() + aa := m.altAdvancements[nodeID] if aa == nil { return nil } return aa.Clone() } -// GetAllAltAdvancements returns a map of all alternate advancements (read-only access) -// The returned map should not be modified +// GetAllAltAdvancements returns a copy of all alternate advancements map func (m *MasterList) GetAllAltAdvancements() map[int32]*AltAdvancement { - return m.MasterList.GetAll() + m.mutex.RLock() + defer m.mutex.RUnlock() + + // Return a copy to prevent external modification + result := make(map[int32]*AltAdvancement, len(m.altAdvancements)) + for id, aa := range m.altAdvancements { + result[id] = aa + } + return result } -// GetAltAdvancementsByGroup returns alternate advancements filtered by group/tab +// GetAltAdvancementsByGroup returns all alternate advancements in a group (O(1)) func (m *MasterList) GetAltAdvancementsByGroup(group int8) []*AltAdvancement { - return m.MasterList.Filter(func(aa *AltAdvancement) bool { - return aa.Group == group - }) + m.mutex.RLock() + defer m.mutex.RUnlock() + return m.byGroup[group] } -// GetAltAdvancementsByClass returns alternate advancements filtered by class requirement +// GetAltAdvancementsByClass returns all alternate advancements for a class (O(1)) func (m *MasterList) GetAltAdvancementsByClass(classID int8) []*AltAdvancement { - return m.MasterList.Filter(func(aa *AltAdvancement) bool { - return aa.ClassReq == 0 || aa.ClassReq == classID - }) + m.mutex.RLock() + defer m.mutex.RUnlock() + + // Return class-specific AAs plus universal AAs (ClassReq == 0) + var result []*AltAdvancement + + // Add class-specific AAs + if classAAs := m.byClass[classID]; classAAs != nil { + result = append(result, classAAs...) + } + + // Add universal AAs (ClassReq == 0) + if universalAAs := m.byClass[0]; universalAAs != nil { + result = append(result, universalAAs...) + } + + return result } -// GetAltAdvancementsByLevel returns alternate advancements available at a specific level +// GetAltAdvancementsByLevel returns all alternate advancements available at a specific level func (m *MasterList) GetAltAdvancementsByLevel(level int8) []*AltAdvancement { - return m.MasterList.Filter(func(aa *AltAdvancement) bool { - return aa.MinLevel <= level - }) + m.mutex.RLock() + defer m.mutex.RUnlock() + + var result []*AltAdvancement + + // Collect all AAs with MinLevel <= level + for minLevel, aas := range m.byLevel { + if minLevel <= level { + result = append(result, aas...) + } + } + + return result } -// RemoveAltAdvancement removes an alternate advancement from the master list -// Returns true if AA was found and removed +// GetAltAdvancementsByGroupAndClass returns AAs matching both group and class +func (m *MasterList) GetAltAdvancementsByGroupAndClass(group, classID int8) []*AltAdvancement { + m.mutex.RLock() + defer m.mutex.RUnlock() + + groupAAs := m.byGroup[group] + if groupAAs == nil { + return nil + } + + var result []*AltAdvancement + for _, aa := range groupAAs { + if aa.ClassReq == 0 || aa.ClassReq == classID { + result = append(result, aa) + } + } + + return result +} + +// GetGroups returns all unique groups using cached results +func (m *MasterList) GetGroups() []int8 { + m.mutex.Lock() // Need write lock to potentially update cache + defer m.mutex.Unlock() + + m.refreshMetaCache() + + // Return a copy to prevent external modification + result := make([]int8, len(m.groups)) + copy(result, m.groups) + return result +} + +// GetClasses returns all unique classes using cached results +func (m *MasterList) GetClasses() []int8 { + m.mutex.Lock() // Need write lock to potentially update cache + defer m.mutex.Unlock() + + m.refreshMetaCache() + + // Return a copy to prevent external modification + result := make([]int8, len(m.classes)) + copy(result, m.classes) + return result +} + +// RemoveAltAdvancement removes an alternate advancement and updates all indices func (m *MasterList) RemoveAltAdvancement(nodeID int32) bool { - return m.MasterList.Remove(nodeID) + m.mutex.Lock() + defer m.mutex.Unlock() + + aa, exists := m.altAdvancements[nodeID] + if !exists { + return false + } + + // Remove from core storage + delete(m.altAdvancements, nodeID) + + // Remove from group index + groupAAs := m.byGroup[aa.Group] + for i, a := range groupAAs { + if a.NodeID == nodeID { + m.byGroup[aa.Group] = append(groupAAs[:i], groupAAs[i+1:]...) + break + } + } + + // Remove from class index + classAAs := m.byClass[aa.ClassReq] + for i, a := range classAAs { + if a.NodeID == nodeID { + m.byClass[aa.ClassReq] = append(classAAs[:i], classAAs[i+1:]...) + break + } + } + + // Remove from level index + levelAAs := m.byLevel[aa.MinLevel] + for i, a := range levelAAs { + if a.NodeID == nodeID { + m.byLevel[aa.MinLevel] = append(levelAAs[:i], levelAAs[i+1:]...) + break + } + } + + // Invalidate metadata cache + m.metaStale = true + + return true } // UpdateAltAdvancement updates an existing alternate advancement -// Returns error if AA doesn't exist func (m *MasterList) UpdateAltAdvancement(aa *AltAdvancement) error { if aa == nil { return fmt.Errorf("alternate advancement cannot be nil") } - return m.MasterList.Update(aa) -} -// GetGroups returns all unique groups/tabs that have alternate advancements -func (m *MasterList) GetGroups() []int8 { - groups := make(map[int8]bool) + m.mutex.Lock() + defer m.mutex.Unlock() - m.MasterList.ForEach(func(_ int32, aa *AltAdvancement) { - groups[aa.Group] = true - }) - - result := make([]int8, 0, len(groups)) - for group := range groups { - result = append(result, group) + // Check if exists + old, exists := m.altAdvancements[aa.NodeID] + if !exists { + return fmt.Errorf("alternate advancement %d not found", aa.NodeID) } - return result -} -// GetClasses returns all unique classes that have alternate advancements -func (m *MasterList) GetClasses() []int8 { - classes := make(map[int8]bool) - - m.MasterList.ForEach(func(_ int32, aa *AltAdvancement) { - if aa.ClassReq > 0 { - classes[aa.ClassReq] = true + // Remove old AA from indices (but not core storage yet) + groupAAs := m.byGroup[old.Group] + for i, a := range groupAAs { + if a.NodeID == aa.NodeID { + m.byGroup[old.Group] = append(groupAAs[:i], groupAAs[i+1:]...) + break } - }) - - result := make([]int8, 0, len(classes)) - for class := range classes { - result = append(result, class) } - return result + + classAAs := m.byClass[old.ClassReq] + for i, a := range classAAs { + if a.NodeID == aa.NodeID { + m.byClass[old.ClassReq] = append(classAAs[:i], classAAs[i+1:]...) + break + } + } + + levelAAs := m.byLevel[old.MinLevel] + for i, a := range levelAAs { + if a.NodeID == aa.NodeID { + m.byLevel[old.MinLevel] = append(levelAAs[:i], levelAAs[i+1:]...) + break + } + } + + // Update core storage + m.altAdvancements[aa.NodeID] = aa + + // Add new AA to indices + m.byGroup[aa.Group] = append(m.byGroup[aa.Group], aa) + m.byClass[aa.ClassReq] = append(m.byClass[aa.ClassReq], aa) + m.byLevel[aa.MinLevel] = append(m.byLevel[aa.MinLevel], aa) + + // Invalidate metadata cache + m.metaStale = true + + return nil +} + +// Size returns the total number of alternate advancements +func (m *MasterList) Size() int { + m.mutex.RLock() + defer m.mutex.RUnlock() + return len(m.altAdvancements) +} + +// Clear removes all alternate advancements from the master list +func (m *MasterList) Clear() { + m.mutex.Lock() + defer m.mutex.Unlock() + + // Clear all maps + m.altAdvancements = make(map[int32]*AltAdvancement) + m.byGroup = make(map[int8][]*AltAdvancement) + m.byClass = make(map[int8][]*AltAdvancement) + m.byLevel = make(map[int8][]*AltAdvancement) + + // Clear cached metadata + m.groups = m.groups[:0] + m.classes = m.classes[:0] + m.metaStale = true +} + +// ForEach executes a function for each alternate advancement +func (m *MasterList) ForEach(fn func(int32, *AltAdvancement)) { + m.mutex.RLock() + defer m.mutex.RUnlock() + + for id, aa := range m.altAdvancements { + fn(id, aa) + } } // ValidateAll validates all alternate advancements in the master list func (m *MasterList) ValidateAll() []error { + m.mutex.RLock() + defer m.mutex.RUnlock() + var errors []error - m.MasterList.ForEach(func(nodeID int32, aa *AltAdvancement) { + for nodeID, aa := range m.altAdvancements { if !aa.IsValid() { errors = append(errors, fmt.Errorf("invalid AA data: node_id=%d", nodeID)) } // Validate prerequisites if aa.RankPrereqID > 0 { - prereq := m.MasterList.Get(aa.RankPrereqID) + prereq := m.altAdvancements[aa.RankPrereqID] if prereq == nil { errors = append(errors, fmt.Errorf("AA %d has invalid prerequisite node ID %d", nodeID, aa.RankPrereqID)) } @@ -151,7 +394,7 @@ func (m *MasterList) ValidateAll() []error { if aa.MaxRank < MIN_MAX_RANK || aa.MaxRank > MAX_MAX_RANK { errors = append(errors, fmt.Errorf("AA %d has invalid max rank %d", nodeID, aa.MaxRank)) } - }) + } return errors }