fix: address code review issues for performance tuning
- commit
- 5bc7d83cec9061eea781462bc4e029b06189339f
- author
- brice · · 2025-12-29 17:20 UTC
- committer
- brice · · 2025-12-29 17:28 UTC
- parent
- e64d6f3
- signature
- unsigned
4 files · +36 -18
M internal/infrastructure/config/profile.go +7 -6
| @@ -12,14 +12,15 @@ const ( | |||
| 12 | 12 | maxSkiaCPUThreads = 8 | |
| 13 | 13 | ||
| 14 | 14 | // Memory tier thresholds (in bytes) | |
| 15 | - | ramTierLow = 8 * 1024 * 1024 * 1024 // 8 GB | |
| 16 | - | ramTierMedium = 16 * 1024 * 1024 * 1024 // 16 GB | |
| 17 | - | ramTierHigh = 32 * 1024 * 1024 * 1024 // 32 GB | |
| 15 | + | // Explicitly typed as uint64 to prevent overflow on 32-bit systems. | |
| 16 | + | ramTierLow uint64 = 8 * 1024 * 1024 * 1024 // 8 GB | |
| 17 | + | ramTierMedium uint64 = 16 * 1024 * 1024 * 1024 // 16 GB | |
| 18 | + | ramTierHigh uint64 = 32 * 1024 * 1024 * 1024 // 32 GB | |
| 18 | 19 | ||
| 19 | 20 | // VRAM tier thresholds (in bytes) | |
| 20 | - | vramTierLow = 4 * 1024 * 1024 * 1024 // 4 GB (low-end discrete / integrated) | |
| 21 | - | vramTierMid = 8 * 1024 * 1024 * 1024 // 8 GB (mid-range discrete) | |
| 22 | - | vramTierHigh = 16 * 1024 * 1024 * 1024 // 16 GB (high-end discrete) | |
| 21 | + | vramTierLow uint64 = 4 * 1024 * 1024 * 1024 // 4 GB (low-end discrete / integrated) | |
| 22 | + | vramTierMid uint64 = 8 * 1024 * 1024 * 1024 // 8 GB (mid-range discrete) | |
| 23 | + | vramTierHigh uint64 = 16 * 1024 * 1024 * 1024 // 16 GB (high-end discrete) | |
| 23 | 24 | ||
| 24 | 25 | // GPU thread scaling values | |
| 25 | 26 | gpuThreadsHigh = 8 // For 16GB+ VRAM | |
| @@ -12,14 +12,15 @@ const ( | |||||
| 12 | maxSkiaCPUThreads = 8 | 12 | maxSkiaCPUThreads = 8 | ||
| 13 | 13 | ||||
| 14 | // Memory tier thresholds (in bytes) | 14 | // Memory tier thresholds (in bytes) | ||
| 15 | - | ramTierLow = 8 * 1024 * 1024 * 1024 // 8 GB | 15 | + | // Explicitly typed as uint64 to prevent overflow on 32-bit systems. |
| 16 | - | ramTierMedium = 16 * 1024 * 1024 * 1024 // 16 GB | 16 | + | ramTierLow uint64 = 8 * 1024 * 1024 * 1024 // 8 GB |
| 17 | - | ramTierHigh = 32 * 1024 * 1024 * 1024 // 32 GB | 17 | + | ramTierMedium uint64 = 16 * 1024 * 1024 * 1024 // 16 GB |
| 18 | + | ramTierHigh uint64 = 32 * 1024 * 1024 * 1024 // 32 GB | |||
| 18 | 19 | ||||
| 19 | // VRAM tier thresholds (in bytes) | 20 | // VRAM tier thresholds (in bytes) | ||
| 20 | - | vramTierLow = 4 * 1024 * 1024 * 1024 // 4 GB (low-end discrete / integrated) | 21 | + | vramTierLow uint64 = 4 * 1024 * 1024 * 1024 // 4 GB (low-end discrete / integrated) |
| 21 | - | vramTierMid = 8 * 1024 * 1024 * 1024 // 8 GB (mid-range discrete) | 22 | + | vramTierMid uint64 = 8 * 1024 * 1024 * 1024 // 8 GB (mid-range discrete) |
| 22 | - | vramTierHigh = 16 * 1024 * 1024 * 1024 // 16 GB (high-end discrete) | 23 | + | vramTierHigh uint64 = 16 * 1024 * 1024 * 1024 // 16 GB (high-end discrete) |
| 23 | 24 | ||||
| 24 | // GPU thread scaling values | 25 | // GPU thread scaling values | ||
| 25 | gpuThreadsHigh = 8 // For 16GB+ VRAM | 26 | gpuThreadsHigh = 8 // For 16GB+ VRAM | ||
M internal/infrastructure/config/webui_gateway.go +16 -5
| @@ -15,6 +15,17 @@ func NewWebUIConfigGateway(mgr *Manager) *WebUIConfigGateway { | |||
| 15 | 15 | return &WebUIConfigGateway{mgr: mgr} | |
| 16 | 16 | } | |
| 17 | 17 | ||
| 18 | + | // clampInt returns v clamped to [min, max]. | |
| 19 | + | func clampInt(v, min, max int) int { | |
| 20 | + | if v < min { | |
| 21 | + | return min | |
| 22 | + | } | |
| 23 | + | if v > max { | |
| 24 | + | return max | |
| 25 | + | } | |
| 26 | + | return v | |
| 27 | + | } | |
| 28 | + | ||
| 18 | 29 | func (g *WebUIConfigGateway) SaveWebUIConfig(ctx context.Context, cfg port.WebUIConfig) error { | |
| 19 | 30 | _ = ctx | |
| 20 | 31 | if g == nil || g.mgr == nil { | |
| @@ -67,11 +78,11 @@ func (g *WebUIConfigGateway) SaveWebUIConfig(ctx context.Context, cfg port.WebUI | |||
| 67 | 78 | ||
| 68 | 79 | // Custom performance fields (only used when profile is "custom") | |
| 69 | 80 | if cfg.Performance.Profile == string(ProfileCustom) { | |
| 70 | - | current.Performance.SkiaCPUPaintingThreads = cfg.Performance.Custom.SkiaCPUThreads | |
| 71 | - | current.Performance.SkiaGPUPaintingThreads = cfg.Performance.Custom.SkiaGPUThreads | |
| 72 | - | current.Performance.WebProcessMemoryLimitMB = cfg.Performance.Custom.WebProcessMemoryMB | |
| 73 | - | current.Performance.NetworkProcessMemoryLimitMB = cfg.Performance.Custom.NetworkProcessMemoryMB | |
| 74 | - | current.Performance.WebViewPoolPrewarmCount = cfg.Performance.Custom.WebViewPoolPrewarm | |
| 81 | + | current.Performance.SkiaCPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaCPUThreads, 0, maxSkiaCPUThreads) | |
| 82 | + | current.Performance.SkiaGPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaGPUThreads, -1, maxSkiaCPUThreads) | |
| 83 | + | current.Performance.WebProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.WebProcessMemoryMB, 0, 16384) | |
| 84 | + | current.Performance.NetworkProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.NetworkProcessMemoryMB, 0, 4096) | |
| 85 | + | current.Performance.WebViewPoolPrewarmCount = clampInt(cfg.Performance.Custom.WebViewPoolPrewarm, 0, 20) | |
| 75 | 86 | } | |
| 76 | 87 | ||
| 77 | 88 | return g.mgr.Save(current) | |
| @@ -15,6 +15,17 @@ func NewWebUIConfigGateway(mgr *Manager) *WebUIConfigGateway { | |||||
| 15 | return &WebUIConfigGateway{mgr: mgr} | 15 | return &WebUIConfigGateway{mgr: mgr} | ||
| 16 | } | 16 | } | ||
| 17 | 17 | ||||
| 18 | + | // clampInt returns v clamped to [min, max]. | |||
| 19 | + | func clampInt(v, min, max int) int { | |||
| 20 | + | if v < min { | |||
| 21 | + | return min | |||
| 22 | + | } | |||
| 23 | + | if v > max { | |||
| 24 | + | return max | |||
| 25 | + | } | |||
| 26 | + | return v | |||
| 27 | + | } | |||
| 28 | + | ||||
| 18 | func (g *WebUIConfigGateway) SaveWebUIConfig(ctx context.Context, cfg port.WebUIConfig) error { | 29 | func (g *WebUIConfigGateway) SaveWebUIConfig(ctx context.Context, cfg port.WebUIConfig) error { | ||
| 19 | _ = ctx | 30 | _ = ctx | ||
| 20 | if g == nil || g.mgr == nil { | 31 | if g == nil || g.mgr == nil { | ||
| @@ -67,11 +78,11 @@ func (g *WebUIConfigGateway) SaveWebUIConfig(ctx context.Context, cfg port.WebUI | |||||
| 67 | 78 | ||||
| 68 | // Custom performance fields (only used when profile is "custom") | 79 | // Custom performance fields (only used when profile is "custom") | ||
| 69 | if cfg.Performance.Profile == string(ProfileCustom) { | 80 | if cfg.Performance.Profile == string(ProfileCustom) { | ||
| 70 | - | current.Performance.SkiaCPUPaintingThreads = cfg.Performance.Custom.SkiaCPUThreads | 81 | + | current.Performance.SkiaCPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaCPUThreads, 0, maxSkiaCPUThreads) |
| 71 | - | current.Performance.SkiaGPUPaintingThreads = cfg.Performance.Custom.SkiaGPUThreads | 82 | + | current.Performance.SkiaGPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaGPUThreads, -1, maxSkiaCPUThreads) |
| 72 | - | current.Performance.WebProcessMemoryLimitMB = cfg.Performance.Custom.WebProcessMemoryMB | 83 | + | current.Performance.WebProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.WebProcessMemoryMB, 0, 16384) |
| 73 | - | current.Performance.NetworkProcessMemoryLimitMB = cfg.Performance.Custom.NetworkProcessMemoryMB | 84 | + | current.Performance.NetworkProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.NetworkProcessMemoryMB, 0, 4096) |
| 74 | - | current.Performance.WebViewPoolPrewarmCount = cfg.Performance.Custom.WebViewPoolPrewarm | 85 | + | current.Performance.WebViewPoolPrewarmCount = clampInt(cfg.Performance.Custom.WebViewPoolPrewarm, 0, 20) |
| 75 | } | 86 | } | ||
| 76 | 87 | ||||
| 77 | return g.mgr.Save(current) | 88 | return g.mgr.Save(current) | ||
M internal/infrastructure/env/hardware_survey.go +12 -6
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "runtime" | |
| 9 | 9 | "strconv" | |
| 10 | 10 | "strings" | |
| 11 | + | "sync" | |
| 11 | 12 | ||
| 12 | 13 | "github.com/bnema/dumber/internal/application/port" | |
| 13 | 14 | "github.com/bnema/dumber/internal/logging" | |
| @@ -19,8 +20,10 @@ const ( | |||
| 19 | 20 | ) | |
| 20 | 21 | ||
| 21 | 22 | // HardwareSurveyor implements port.HardwareSurveyor for Linux systems. | |
| 23 | + | // It is safe for concurrent use after creation. | |
| 22 | 24 | type HardwareSurveyor struct { | |
| 23 | - | cached *port.HardwareInfo | |
| 25 | + | once sync.Once | |
| 26 | + | cached port.HardwareInfo | |
| 24 | 27 | } | |
| 25 | 28 | ||
| 26 | 29 | // NewHardwareSurveyor creates a new hardware surveyor. | |
| @@ -29,12 +32,16 @@ func NewHardwareSurveyor() *HardwareSurveyor { | |||
| 29 | 32 | } | |
| 30 | 33 | ||
| 31 | 34 | // Survey detects and returns hardware information. | |
| 32 | - | // Results are cached after the first call. | |
| 35 | + | // Results are cached after the first call. Safe for concurrent use. | |
| 33 | 36 | func (s *HardwareSurveyor) Survey(ctx context.Context) port.HardwareInfo { | |
| 34 | - | if s.cached != nil { | |
| 35 | - | return *s.cached | |
| 36 | - | } | |
| 37 | + | s.once.Do(func() { | |
| 38 | + | s.cached = s.doSurvey(ctx) | |
| 39 | + | }) | |
| 40 | + | return s.cached | |
| 41 | + | } | |
| 37 | 42 | ||
| 43 | + | // doSurvey performs the actual hardware detection. | |
| 44 | + | func (s *HardwareSurveyor) doSurvey(ctx context.Context) port.HardwareInfo { | |
| 38 | 45 | log := logging.FromContext(ctx) | |
| 39 | 46 | ||
| 40 | 47 | info := port.HardwareInfo{ | |
| @@ -63,7 +70,6 @@ func (s *HardwareSurveyor) Survey(ctx context.Context) port.HardwareInfo { | |||
| 63 | 70 | Uint64("vram_mb", info.VRAM/(1024*1024)). | |
| 64 | 71 | Msg("hardware survey completed") | |
| 65 | 72 | ||
| 66 | - | s.cached = &info | |
| 67 | 73 | return info | |
| 68 | 74 | } | |
| 69 | 75 | ||
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "runtime" | 8 | "runtime" | ||
| 9 | "strconv" | 9 | "strconv" | ||
| 10 | "strings" | 10 | "strings" | ||
| 11 | + | "sync" | |||
| 11 | 12 | ||||
| 12 | "github.com/bnema/dumber/internal/application/port" | 13 | "github.com/bnema/dumber/internal/application/port" | ||
| 13 | "github.com/bnema/dumber/internal/logging" | 14 | "github.com/bnema/dumber/internal/logging" | ||
| @@ -19,8 +20,10 @@ const ( | |||||
| 19 | ) | 20 | ) | ||
| 20 | 21 | ||||
| 21 | // HardwareSurveyor implements port.HardwareSurveyor for Linux systems. | 22 | // HardwareSurveyor implements port.HardwareSurveyor for Linux systems. | ||
| 23 | + | // It is safe for concurrent use after creation. | |||
| 22 | type HardwareSurveyor struct { | 24 | type HardwareSurveyor struct { | ||
| 23 | - | cached *port.HardwareInfo | 25 | + | once sync.Once |
| 26 | + | cached port.HardwareInfo | |||
| 24 | } | 27 | } | ||
| 25 | 28 | ||||
| 26 | // NewHardwareSurveyor creates a new hardware surveyor. | 29 | // NewHardwareSurveyor creates a new hardware surveyor. | ||
| @@ -29,12 +32,16 @@ func NewHardwareSurveyor() *HardwareSurveyor { | |||||
| 29 | } | 32 | } | ||
| 30 | 33 | ||||
| 31 | // Survey detects and returns hardware information. | 34 | // Survey detects and returns hardware information. | ||
| 32 | - | // Results are cached after the first call. | 35 | + | // Results are cached after the first call. Safe for concurrent use. |
| 33 | func (s *HardwareSurveyor) Survey(ctx context.Context) port.HardwareInfo { | 36 | func (s *HardwareSurveyor) Survey(ctx context.Context) port.HardwareInfo { | ||
| 34 | - | if s.cached != nil { | 37 | + | s.once.Do(func() { |
| 35 | - | return *s.cached | 38 | + | s.cached = s.doSurvey(ctx) |
| 36 | - | } | 39 | + | }) |
| 40 | + | return s.cached | |||
| 41 | + | } | |||
| 37 | 42 | ||||
| 43 | + | // doSurvey performs the actual hardware detection. | |||
| 44 | + | func (s *HardwareSurveyor) doSurvey(ctx context.Context) port.HardwareInfo { | |||
| 38 | log := logging.FromContext(ctx) | 45 | log := logging.FromContext(ctx) | ||
| 39 | 46 | ||||
| 40 | info := port.HardwareInfo{ | 47 | info := port.HardwareInfo{ | ||
| @@ -63,7 +70,6 @@ func (s *HardwareSurveyor) Survey(ctx context.Context) port.HardwareInfo { | |||||
| 63 | Uint64("vram_mb", info.VRAM/(1024*1024)). | 70 | Uint64("vram_mb", info.VRAM/(1024*1024)). | ||
| 64 | Msg("hardware survey completed") | 71 | Msg("hardware survey completed") | ||
| 65 | 72 | ||||
| 66 | - | s.cached = &info | |||
| 67 | return info | 73 | return info | ||
| 68 | } | 74 | } | ||
| 69 | 75 | ||||
M webui/src/pages/ConfigPage.svelte +1 -1
| @@ -446,7 +446,7 @@ | |||
| 446 | 446 | <div><span class="text-muted-foreground">CPU:</span> <span class="font-mono">{hw.cpu_cores}c/{hw.cpu_threads}t</span></div> | |
| 447 | 447 | <div><span class="text-muted-foreground">RAM:</span> <span class="font-mono">{hw.total_ram_mb > 0 ? (hw.total_ram_mb / 1024).toFixed(0) + ' GB' : '?'}</span></div> | |
| 448 | 448 | <div><span class="text-muted-foreground">GPU:</span> <span class="font-mono capitalize">{hw.gpu_vendor || 'Unknown'}</span></div> | |
| 449 | - | <div><span class="text-muted-foreground">VRAM:</span> <span class="font-mono">{hw.vram_mb > 0 ? (hw.vram_mb / 1024).toFixed(0) + ' GB' : '?'}</span></div> | |
| 449 | + | <div><span class="text-muted-foreground">VRAM:</span> <span class="font-mono">{hw.vram_mb > 0 ? (hw.vram_mb / 1024).toFixed(0) + ' GB' : (hw.gpu_vendor === 'intel' ? 'Shared' : '?')}</span></div> | |
| 450 | 450 | </div> | |
| 451 | 451 | </div> | |
| 452 | 452 | {/if} | |
| @@ -446,7 +446,7 @@ | |||||
| 446 | <div><span class="text-muted-foreground">CPU:</span> <span class="font-mono">{hw.cpu_cores}c/{hw.cpu_threads}t</span></div> | 446 | <div><span class="text-muted-foreground">CPU:</span> <span class="font-mono">{hw.cpu_cores}c/{hw.cpu_threads}t</span></div> | ||
| 447 | <div><span class="text-muted-foreground">RAM:</span> <span class="font-mono">{hw.total_ram_mb > 0 ? (hw.total_ram_mb / 1024).toFixed(0) + ' GB' : '?'}</span></div> | 447 | <div><span class="text-muted-foreground">RAM:</span> <span class="font-mono">{hw.total_ram_mb > 0 ? (hw.total_ram_mb / 1024).toFixed(0) + ' GB' : '?'}</span></div> | ||
| 448 | <div><span class="text-muted-foreground">GPU:</span> <span class="font-mono capitalize">{hw.gpu_vendor || 'Unknown'}</span></div> | 448 | <div><span class="text-muted-foreground">GPU:</span> <span class="font-mono capitalize">{hw.gpu_vendor || 'Unknown'}</span></div> | ||
| 449 | - | <div><span class="text-muted-foreground">VRAM:</span> <span class="font-mono">{hw.vram_mb > 0 ? (hw.vram_mb / 1024).toFixed(0) + ' GB' : '?'}</span></div> | 449 | + | <div><span class="text-muted-foreground">VRAM:</span> <span class="font-mono">{hw.vram_mb > 0 ? (hw.vram_mb / 1024).toFixed(0) + ' GB' : (hw.gpu_vendor === 'intel' ? 'Shared' : '?')}</span></div> |
| 450 | </div> | 450 | </div> | ||
| 451 | </div> | 451 | </div> | ||
| 452 | {/if} | 452 | {/if} | ||