fix: address Copilot review feedback for performance tuning
- commit
- 6e1d371705a0854ca39c2d7728326cea688d6be8
- author
- brice · · 2025-12-29 17:42 UTC
- parent
- b5135b0
- signature
- unsigned
6 files · +43 -4
M internal/application/port/hardware.go +12 -2
| @@ -37,13 +37,23 @@ func (h HardwareInfo) CPUThreadsOrDefault(fallback int) int { | |||
| 37 | 37 | } | |
| 38 | 38 | ||
| 39 | 39 | // TotalRAMMB returns total RAM in megabytes. | |
| 40 | + | // Returns 0 if RAM exceeds int range (unlikely on any real system). | |
| 40 | 41 | func (h HardwareInfo) TotalRAMMB() int { | |
| 41 | - | return int(h.TotalRAM / (1024 * 1024)) | |
| 42 | + | mb := h.TotalRAM / (1024 * 1024) | |
| 43 | + | if mb > uint64(^uint(0)>>1) { // Check against max int | |
| 44 | + | return 0 | |
| 45 | + | } | |
| 46 | + | return int(mb) | |
| 42 | 47 | } | |
| 43 | 48 | ||
| 44 | 49 | // VRAMMB returns VRAM in megabytes. | |
| 50 | + | // Returns 0 if VRAM exceeds int range (unlikely on any real system). | |
| 45 | 51 | func (h HardwareInfo) VRAMMB() int { | |
| 46 | - | return int(h.VRAM / (1024 * 1024)) | |
| 52 | + | mb := h.VRAM / (1024 * 1024) | |
| 53 | + | if mb > uint64(^uint(0)>>1) { // Check against max int | |
| 54 | + | return 0 | |
| 55 | + | } | |
| 56 | + | return int(mb) | |
| 47 | 57 | } | |
| 48 | 58 | ||
| 49 | 59 | // HasDedicatedGPU returns true if a discrete GPU with VRAM was detected. | |
| @@ -37,13 +37,23 @@ func (h HardwareInfo) CPUThreadsOrDefault(fallback int) int { | |||||
| 37 | } | 37 | } | ||
| 38 | 38 | ||||
| 39 | // TotalRAMMB returns total RAM in megabytes. | 39 | // TotalRAMMB returns total RAM in megabytes. | ||
| 40 | + | // Returns 0 if RAM exceeds int range (unlikely on any real system). | |||
| 40 | func (h HardwareInfo) TotalRAMMB() int { | 41 | func (h HardwareInfo) TotalRAMMB() int { | ||
| 41 | - | return int(h.TotalRAM / (1024 * 1024)) | 42 | + | mb := h.TotalRAM / (1024 * 1024) |
| 43 | + | if mb > uint64(^uint(0)>>1) { // Check against max int | |||
| 44 | + | return 0 | |||
| 45 | + | } | |||
| 46 | + | return int(mb) | |||
| 42 | } | 47 | } | ||
| 43 | 48 | ||||
| 44 | // VRAMMB returns VRAM in megabytes. | 49 | // VRAMMB returns VRAM in megabytes. | ||
| 50 | + | // Returns 0 if VRAM exceeds int range (unlikely on any real system). | |||
| 45 | func (h HardwareInfo) VRAMMB() int { | 51 | func (h HardwareInfo) VRAMMB() int { | ||
| 46 | - | return int(h.VRAM / (1024 * 1024)) | 52 | + | mb := h.VRAM / (1024 * 1024) |
| 53 | + | if mb > uint64(^uint(0)>>1) { // Check against max int | |||
| 54 | + | return 0 | |||
| 55 | + | } | |||
| 56 | + | return int(mb) | |||
| 47 | } | 57 | } | ||
| 48 | 58 | ||||
| 49 | // HasDedicatedGPU returns true if a discrete GPU with VRAM was detected. | 59 | // HasDedicatedGPU returns true if a discrete GPU with VRAM was detected. | ||
M internal/application/port/webkit_context.go +6 -0
| @@ -58,11 +58,17 @@ type WebKitContextOptions struct { | |||
| 58 | 58 | ||
| 59 | 59 | // IsWebProcessMemoryConfigured returns true if web process memory settings are configured. | |
| 60 | 60 | func (o *WebKitContextOptions) IsWebProcessMemoryConfigured() bool { | |
| 61 | + | if o == nil { | |
| 62 | + | return false | |
| 63 | + | } | |
| 61 | 64 | return o.WebProcessMemory.IsConfigured() | |
| 62 | 65 | } | |
| 63 | 66 | ||
| 64 | 67 | // IsNetworkProcessMemoryConfigured returns true if network process memory settings are configured. | |
| 65 | 68 | func (o *WebKitContextOptions) IsNetworkProcessMemoryConfigured() bool { | |
| 69 | + | if o == nil { | |
| 70 | + | return false | |
| 71 | + | } | |
| 66 | 72 | return o.NetworkProcessMemory.IsConfigured() | |
| 67 | 73 | } | |
| 68 | 74 | ||
| @@ -58,11 +58,17 @@ type WebKitContextOptions struct { | |||||
| 58 | 58 | ||||
| 59 | // IsWebProcessMemoryConfigured returns true if web process memory settings are configured. | 59 | // IsWebProcessMemoryConfigured returns true if web process memory settings are configured. | ||
| 60 | func (o *WebKitContextOptions) IsWebProcessMemoryConfigured() bool { | 60 | func (o *WebKitContextOptions) IsWebProcessMemoryConfigured() bool { | ||
| 61 | + | if o == nil { | |||
| 62 | + | return false | |||
| 63 | + | } | |||
| 61 | return o.WebProcessMemory.IsConfigured() | 64 | return o.WebProcessMemory.IsConfigured() | ||
| 62 | } | 65 | } | ||
| 63 | 66 | ||||
| 64 | // IsNetworkProcessMemoryConfigured returns true if network process memory settings are configured. | 67 | // IsNetworkProcessMemoryConfigured returns true if network process memory settings are configured. | ||
| 65 | func (o *WebKitContextOptions) IsNetworkProcessMemoryConfigured() bool { | 68 | func (o *WebKitContextOptions) IsNetworkProcessMemoryConfigured() bool { | ||
| 69 | + | if o == nil { | |||
| 70 | + | return false | |||
| 71 | + | } | |||
| 66 | return o.NetworkProcessMemory.IsConfigured() | 72 | return o.NetworkProcessMemory.IsConfigured() | ||
| 67 | } | 73 | } | ||
| 68 | 74 | ||||
M internal/infrastructure/config/profile.go +4 -0
| @@ -11,6 +11,10 @@ const ( | |||
| 11 | 11 | // supported by WebKitGTK (0-8 range). | |
| 12 | 12 | maxSkiaCPUThreads = 8 | |
| 13 | 13 | ||
| 14 | + | // maxSkiaGPUThreads is the maximum number of Skia GPU painting threads | |
| 15 | + | // supported by WebKitGTK (-1 to 8 range, where -1 means "let WebKit decide"). | |
| 16 | + | maxSkiaGPUThreads = 8 | |
| 17 | + | ||
| 14 | 18 | // Memory tier thresholds (in bytes) | |
| 15 | 19 | // Explicitly typed as uint64 to prevent overflow on 32-bit systems. | |
| 16 | 20 | ramTierLow uint64 = 8 * 1024 * 1024 * 1024 // 8 GB | |
| @@ -11,6 +11,10 @@ const ( | |||||
| 11 | // supported by WebKitGTK (0-8 range). | 11 | // supported by WebKitGTK (0-8 range). | ||
| 12 | maxSkiaCPUThreads = 8 | 12 | maxSkiaCPUThreads = 8 | ||
| 13 | 13 | ||||
| 14 | + | // maxSkiaGPUThreads is the maximum number of Skia GPU painting threads | |||
| 15 | + | // supported by WebKitGTK (-1 to 8 range, where -1 means "let WebKit decide"). | |||
| 16 | + | maxSkiaGPUThreads = 8 | |||
| 17 | + | ||||
| 14 | // Memory tier thresholds (in bytes) | 18 | // Memory tier thresholds (in bytes) | ||
| 15 | // Explicitly typed as uint64 to prevent overflow on 32-bit systems. | 19 | // Explicitly typed as uint64 to prevent overflow on 32-bit systems. | ||
| 16 | ramTierLow uint64 = 8 * 1024 * 1024 * 1024 // 8 GB | 20 | ramTierLow uint64 = 8 * 1024 * 1024 * 1024 // 8 GB | ||
M internal/infrastructure/config/validation.go +19 -0
| @@ -371,5 +371,24 @@ func validatePerformanceProfile(config *Config) []string { | |||
| 371 | 371 | } | |
| 372 | 372 | } | |
| 373 | 373 | ||
| 374 | + | // Validate memory pressure threshold ordering: WebKit requires conservative < strict | |
| 375 | + | webCons := config.Performance.WebProcessMemoryConservativeThreshold | |
| 376 | + | webStrict := config.Performance.WebProcessMemoryStrictThreshold | |
| 377 | + | if webCons > 0 && webStrict > 0 && webCons >= webStrict { | |
| 378 | + | validationErrors = append(validationErrors, | |
| 379 | + | "performance.web_process_memory_conservative_threshold must be less than "+ | |
| 380 | + | "web_process_memory_strict_threshold when both are set", | |
| 381 | + | ) | |
| 382 | + | } | |
| 383 | + | ||
| 384 | + | netCons := config.Performance.NetworkProcessMemoryConservativeThreshold | |
| 385 | + | netStrict := config.Performance.NetworkProcessMemoryStrictThreshold | |
| 386 | + | if netCons > 0 && netStrict > 0 && netCons >= netStrict { | |
| 387 | + | validationErrors = append(validationErrors, | |
| 388 | + | "performance.network_process_memory_conservative_threshold must be less than "+ | |
| 389 | + | "network_process_memory_strict_threshold when both are set", | |
| 390 | + | ) | |
| 391 | + | } | |
| 392 | + | ||
| 374 | 393 | return validationErrors | |
| 375 | 394 | } | |
| @@ -371,5 +371,24 @@ func validatePerformanceProfile(config *Config) []string { | |||||
| 371 | } | 371 | } | ||
| 372 | } | 372 | } | ||
| 373 | 373 | ||||
| 374 | + | // Validate memory pressure threshold ordering: WebKit requires conservative < strict | |||
| 375 | + | webCons := config.Performance.WebProcessMemoryConservativeThreshold | |||
| 376 | + | webStrict := config.Performance.WebProcessMemoryStrictThreshold | |||
| 377 | + | if webCons > 0 && webStrict > 0 && webCons >= webStrict { | |||
| 378 | + | validationErrors = append(validationErrors, | |||
| 379 | + | "performance.web_process_memory_conservative_threshold must be less than "+ | |||
| 380 | + | "web_process_memory_strict_threshold when both are set", | |||
| 381 | + | ) | |||
| 382 | + | } | |||
| 383 | + | ||||
| 384 | + | netCons := config.Performance.NetworkProcessMemoryConservativeThreshold | |||
| 385 | + | netStrict := config.Performance.NetworkProcessMemoryStrictThreshold | |||
| 386 | + | if netCons > 0 && netStrict > 0 && netCons >= netStrict { | |||
| 387 | + | validationErrors = append(validationErrors, | |||
| 388 | + | "performance.network_process_memory_conservative_threshold must be less than "+ | |||
| 389 | + | "network_process_memory_strict_threshold when both are set", | |||
| 390 | + | ) | |||
| 391 | + | } | |||
| 392 | + | ||||
| 374 | return validationErrors | 393 | return validationErrors | ||
| 375 | } | 394 | } | ||
M internal/infrastructure/config/webui_gateway.go +1 -1
| @@ -86,7 +86,7 @@ func (g *WebUIConfigGateway) SaveWebUIConfig(ctx context.Context, cfg port.WebUI | |||
| 86 | 86 | // Custom performance fields (only used when profile is "custom") | |
| 87 | 87 | if cfg.Performance.Profile == string(ProfileCustom) { | |
| 88 | 88 | current.Performance.SkiaCPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaCPUThreads, 0, maxSkiaCPUThreads) | |
| 89 | - | current.Performance.SkiaGPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaGPUThreads, -1, maxSkiaCPUThreads) | |
| 89 | + | current.Performance.SkiaGPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaGPUThreads, -1, maxSkiaGPUThreads) | |
| 90 | 90 | current.Performance.WebProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.WebProcessMemoryMB, 0, maxWebProcessMemoryMB) | |
| 91 | 91 | current.Performance.NetworkProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.NetworkProcessMemoryMB, 0, maxNetworkProcessMemoryMB) | |
| 92 | 92 | current.Performance.WebViewPoolPrewarmCount = clampInt(cfg.Performance.Custom.WebViewPoolPrewarm, 0, maxWebViewPoolPrewarm) | |
| @@ -86,7 +86,7 @@ func (g *WebUIConfigGateway) SaveWebUIConfig(ctx context.Context, cfg port.WebUI | |||||
| 86 | // Custom performance fields (only used when profile is "custom") | 86 | // Custom performance fields (only used when profile is "custom") | ||
| 87 | if cfg.Performance.Profile == string(ProfileCustom) { | 87 | if cfg.Performance.Profile == string(ProfileCustom) { | ||
| 88 | current.Performance.SkiaCPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaCPUThreads, 0, maxSkiaCPUThreads) | 88 | current.Performance.SkiaCPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaCPUThreads, 0, maxSkiaCPUThreads) | ||
| 89 | - | current.Performance.SkiaGPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaGPUThreads, -1, maxSkiaCPUThreads) | 89 | + | current.Performance.SkiaGPUPaintingThreads = clampInt(cfg.Performance.Custom.SkiaGPUThreads, -1, maxSkiaGPUThreads) |
| 90 | current.Performance.WebProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.WebProcessMemoryMB, 0, maxWebProcessMemoryMB) | 90 | current.Performance.WebProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.WebProcessMemoryMB, 0, maxWebProcessMemoryMB) | ||
| 91 | current.Performance.NetworkProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.NetworkProcessMemoryMB, 0, maxNetworkProcessMemoryMB) | 91 | current.Performance.NetworkProcessMemoryLimitMB = clampInt(cfg.Performance.Custom.NetworkProcessMemoryMB, 0, maxNetworkProcessMemoryMB) | ||
| 92 | current.Performance.WebViewPoolPrewarmCount = clampInt(cfg.Performance.Custom.WebViewPoolPrewarm, 0, maxWebViewPoolPrewarm) | 92 | current.Performance.WebViewPoolPrewarmCount = clampInt(cfg.Performance.Custom.WebViewPoolPrewarm, 0, maxWebViewPoolPrewarm) | ||
M internal/infrastructure/webkit/scheme_handler.go +1 -1
| @@ -539,7 +539,7 @@ func (h *DumbSchemeHandler) RegisterWithContext(wkCtx *WebKitContext) { | |||
| 539 | 539 | ||
| 540 | 540 | // Also register on custom WebContext if different from default | |
| 541 | 541 | customCtx := wkCtx.Context() | |
| 542 | - | if customCtx.GoPointer() != 0 && (defaultCtx == nil || customCtx.GoPointer() != defaultCtx.GoPointer()) { | |
| 542 | + | if customCtx != nil && customCtx.GoPointer() != 0 && (defaultCtx == nil || customCtx.GoPointer() != defaultCtx.GoPointer()) { | |
| 543 | 543 | wkCtx.Context().RegisterUriScheme("dumb", &callback, 0, nil) | |
| 544 | 544 | ||
| 545 | 545 | if secMgr := customCtx.GetSecurityManager(); secMgr != nil { | |
| @@ -539,7 +539,7 @@ func (h *DumbSchemeHandler) RegisterWithContext(wkCtx *WebKitContext) { | |||||
| 539 | 539 | ||||
| 540 | // Also register on custom WebContext if different from default | 540 | // Also register on custom WebContext if different from default | ||
| 541 | customCtx := wkCtx.Context() | 541 | customCtx := wkCtx.Context() | ||
| 542 | - | if customCtx.GoPointer() != 0 && (defaultCtx == nil || customCtx.GoPointer() != defaultCtx.GoPointer()) { | 542 | + | if customCtx != nil && customCtx.GoPointer() != 0 && (defaultCtx == nil || customCtx.GoPointer() != defaultCtx.GoPointer()) { |
| 543 | wkCtx.Context().RegisterUriScheme("dumb", &callback, 0, nil) | 543 | wkCtx.Context().RegisterUriScheme("dumb", &callback, 0, nil) | ||
| 544 | 544 | ||||
| 545 | if secMgr := customCtx.GetSecurityManager(); secMgr != nil { | 545 | if secMgr := customCtx.GetSecurityManager(); secMgr != nil { | ||