diff --git a/internal/factions/benchmark_test.go b/internal/factions/benchmark_test.go index badb892..ec09004 100644 --- a/internal/factions/benchmark_test.go +++ b/internal/factions/benchmark_test.go @@ -6,15 +6,14 @@ import ( // Benchmark MasterFactionList operations func BenchmarkMasterFactionList(b *testing.B) { - mfl := NewMasterList() - - // Pre-populate with factions - for i := int32(1); i <= 1000; i++ { - faction := NewFaction(i, "Benchmark Faction", "Test", "Benchmark test") - mfl.AddFaction(faction) - } - b.Run("GetFaction", func(b *testing.B) { + mfl := NewMasterList() + // Pre-populate with factions + for i := int32(1); i <= 1000; i++ { + faction := NewFaction(i, "Benchmark Faction", "Test", "Benchmark test") + mfl.AddFaction(faction) + } + b.ResetTimer() for i := 0; i < b.N; i++ { factionID := int32((i % 1000) + 1) @@ -23,6 +22,13 @@ func BenchmarkMasterFactionList(b *testing.B) { }) b.Run("GetFactionByName", func(b *testing.B) { + mfl := NewMasterList() + // Pre-populate with factions + for i := int32(1); i <= 1000; i++ { + faction := NewFaction(i, "Benchmark Faction", "Test", "Benchmark test") + mfl.AddFaction(faction) + } + b.ResetTimer() for i := 0; i < b.N; i++ { _ = mfl.GetFactionByName("Benchmark Faction") @@ -30,6 +36,13 @@ func BenchmarkMasterFactionList(b *testing.B) { }) b.Run("AddFaction", func(b *testing.B) { + mfl := NewMasterList() + // Pre-populate with factions + for i := int32(1); i <= 1000; i++ { + faction := NewFaction(i, "Benchmark Faction", "Test", "Benchmark test") + mfl.AddFaction(faction) + } + b.ResetTimer() for i := 0; i < b.N; i++ { factionID := int32(2000 + i) @@ -39,6 +52,13 @@ func BenchmarkMasterFactionList(b *testing.B) { }) b.Run("GetFactionCount", func(b *testing.B) { + mfl := NewMasterList() + // Pre-populate with factions + for i := int32(1); i <= 1000; i++ { + faction := NewFaction(i, "Benchmark Faction", "Test", "Benchmark test") + mfl.AddFaction(faction) + } + b.ResetTimer() for i := 0; i < b.N; i++ { _ = mfl.GetFactionCount() @@ -46,6 +66,13 @@ func BenchmarkMasterFactionList(b *testing.B) { }) b.Run("AddHostileFaction", func(b *testing.B) { + mfl := NewMasterList() + // Pre-populate with factions + for i := int32(1); i <= 1000; i++ { + faction := NewFaction(i, "Benchmark Faction", "Test", "Benchmark test") + mfl.AddFaction(faction) + } + b.ResetTimer() for i := 0; i < b.N; i++ { factionID := int32((i % 1000) + 1) @@ -55,24 +82,41 @@ func BenchmarkMasterFactionList(b *testing.B) { }) b.Run("GetHostileFactions", func(b *testing.B) { + mfl := NewMasterList() + // Pre-populate with factions and some hostile relationships + for i := int32(1); i <= 1000; i++ { + faction := NewFaction(i, "Benchmark Faction", "Test", "Benchmark test") + mfl.AddFaction(faction) + } + // Add a reasonable number of hostile relationships + for i := int32(1); i <= 100; i++ { + for j := int32(1); j <= 5; j++ { + mfl.AddHostileFaction(i, i+j) + } + } + b.ResetTimer() for i := 0; i < b.N; i++ { - factionID := int32((i % 1000) + 1) + factionID := int32((i % 100) + 1) _ = mfl.GetHostileFactions(factionID) } }) b.Run("ValidateFactions", func(b *testing.B) { + mfl := NewMasterList() + // Pre-populate with factions + for i := int32(1); i <= 1000; i++ { + faction := NewFaction(i, "Benchmark Faction", "Test", "Benchmark test") + mfl.AddFaction(faction) + } + // Add some hostile relationships for realistic validation + for i := int32(1); i <= 100; i++ { + mfl.AddHostileFaction(i, i+1) + } + b.ResetTimer() - // Validation is expensive - in real usage this would happen infrequently - // Simulate realistic usage by only validating a few times regardless of b.N - validationCount := 0 - maxValidations := 5 // Only validate 5 times max per benchmark run for i := 0; i < b.N; i++ { - if validationCount < maxValidations && i%(b.N/maxValidations+1) == 0 { - _ = mfl.ValidateFactions() - validationCount++ - } + _ = mfl.ValidateFactions() } }) } @@ -215,15 +259,8 @@ func BenchmarkManager(b *testing.B) { b.Run("ValidateAllFactions", func(b *testing.B) { b.ResetTimer() - // Validation is expensive - in real usage this would happen infrequently - // Simulate realistic usage by only validating a few times regardless of b.N - validationCount := 0 - maxValidations := 5 // Only validate 5 times max per benchmark run for i := 0; i < b.N; i++ { - if validationCount < maxValidations && i%(b.N/maxValidations+1) == 0 { - _ = manager.ValidateAllFactions() - validationCount++ - } + _ = manager.ValidateAllFactions() } }) } diff --git a/internal/factions/master.go b/internal/factions/master.go index ae91d21..c82901c 100644 --- a/internal/factions/master.go +++ b/internal/factions/master.go @@ -269,65 +269,104 @@ func (ml *MasterList) ValidateFactions() []string { ml.mutex.RLock() defer ml.mutex.RUnlock() - issues := make([]string, 0, 10) - allFactions := ml.MasterList.GetAll() + var issues []string + + // Use WithReadLock to avoid copying the entire map + var seenIDs map[int32]*Faction + ml.MasterList.WithReadLock(func(allFactions map[int32]*Faction) { + seenIDs = make(map[int32]*Faction, len(allFactions)) - seenIDs := make(map[int32]*Faction, len(allFactions)) - seenNames := make(map[string]*Faction, len(ml.factionNameList)) + // Pass 1: Validate main faction list and build seenID map + for id, faction := range allFactions { + if faction == nil { + if issues == nil { + issues = make([]string, 0, 10) + } + issues = append(issues, fmt.Sprintf("Faction ID %d is nil", id)) + continue + } - // Pass 1: Validate main faction list and build seenID map - for id, faction := range allFactions { - if faction == nil { - issues = append(issues, fmt.Sprintf("Faction ID %d is nil", id)) - continue + if faction.ID <= 0 || faction.Name == "" { + if issues == nil { + issues = make([]string, 0, 10) + } + issues = append(issues, fmt.Sprintf("Faction ID %d is invalid or unnamed", id)) + } + + if faction.ID != id { + if issues == nil { + issues = make([]string, 0, 10) + } + issues = append(issues, fmt.Sprintf("Faction ID mismatch: map key %d != faction ID %d", id, faction.ID)) + } + + seenIDs[id] = faction } + }) - if faction.ID <= 0 || faction.Name == "" { - issues = append(issues, fmt.Sprintf("Faction ID %d is invalid or unnamed", id)) - } - - if faction.ID != id { - issues = append(issues, fmt.Sprintf("Faction ID mismatch: map key %d != faction ID %d", id, faction.ID)) - } - - seenIDs[id] = faction - } - - // Pass 2: Validate factionNameList and build seenName map + // Pass 2: Validate factionNameList for name, faction := range ml.factionNameList { if faction == nil { + if issues == nil { + issues = make([]string, 0, 10) + } issues = append(issues, fmt.Sprintf("Faction name '%s' maps to nil", name)) continue } if faction.Name != name { + if issues == nil { + issues = make([]string, 0, 10) + } issues = append(issues, fmt.Sprintf("Faction name mismatch: map key '%s' != faction name '%s'", name, faction.Name)) } if _, ok := seenIDs[faction.ID]; !ok { + if issues == nil { + issues = make([]string, 0, 10) + } issues = append(issues, fmt.Sprintf("Faction '%s' (ID %d) exists in name map but not in ID map", name, faction.ID)) } - - seenNames[name] = faction } // Pass 3: Validate relationships using prebuilt seenIDs - validateRelations := func(relations map[int32][]int32, relType string) { - for sourceID, targets := range relations { - if _, ok := seenIDs[sourceID]; !ok { - issues = append(issues, fmt.Sprintf("%s relationship defined for non-existent faction %d", relType, sourceID)) + for sourceID, targets := range ml.hostileFactions { + if _, ok := seenIDs[sourceID]; !ok { + if issues == nil { + issues = make([]string, 0, 10) } - for _, targetID := range targets { - if _, ok := seenIDs[targetID]; !ok { - issues = append(issues, fmt.Sprintf("Faction %d has %s relationship with non-existent faction %d", sourceID, relType, targetID)) + issues = append(issues, fmt.Sprintf("Hostile relationship defined for non-existent faction %d", sourceID)) + } + for _, targetID := range targets { + if _, ok := seenIDs[targetID]; !ok { + if issues == nil { + issues = make([]string, 0, 10) } + issues = append(issues, fmt.Sprintf("Faction %d has Hostile relationship with non-existent faction %d", sourceID, targetID)) } } } - validateRelations(ml.hostileFactions, "Hostile") - validateRelations(ml.friendlyFactions, "Friendly") + for sourceID, targets := range ml.friendlyFactions { + if _, ok := seenIDs[sourceID]; !ok { + if issues == nil { + issues = make([]string, 0, 10) + } + issues = append(issues, fmt.Sprintf("Friendly relationship defined for non-existent faction %d", sourceID)) + } + for _, targetID := range targets { + if _, ok := seenIDs[targetID]; !ok { + if issues == nil { + issues = make([]string, 0, 10) + } + issues = append(issues, fmt.Sprintf("Faction %d has Friendly relationship with non-existent faction %d", sourceID, targetID)) + } + } + } + if issues == nil { + return []string{} + } return issues }