Skip to content

perf: reduce hot-path allocations and defer default env gating to scan [IDE-1786]#1207

Open
bastiandoetsch wants to merge 4 commits intorefactor/IDE-1786_folder-config-refactoringfrom
performance/improve-memory-cpu-usage
Open

perf: reduce hot-path allocations and defer default env gating to scan [IDE-1786]#1207
bastiandoetsch wants to merge 4 commits intorefactor/IDE-1786_folder-config-refactoringfrom
performance/improve-memory-cpu-usage

Conversation

@bastiandoetsch
Copy link
Copy Markdown
Collaborator

@bastiandoetsch bastiandoetsch commented Apr 14, 2026

User description

Description

This PR targets refactor/IDE-1786_folder-config-refactoring and adds performance improvements identified from profiling (config resolution, scan state aggregation, OSS conversion, startup path).

Behavioural notes

  • WaitForDefaultEnv runs in DelegatingConcurrentScanner before scans, not in SnykCli.getCommand / extension executor — faster init, scans still wait for PATH/JAVA_HOME/Maven defaults.
  • UnmarshallOssJson errors report input length instead of embedding the full payload (avoids huge error strings).

Technical summary

  • ConfigResolver: single RLock + getBoolLocked for DisplayableIssueTypesForFolder
  • ScanStateAggregator: per-folder issue-type cache; pre-sized maps
  • OSS: byte prefix for JSON array check; issuesByID index; GetExtendedMessage(format, ...)
  • Hover: channel buffer 10000 → 100
  • Benchmarks: config resolver, StateSnapshot, UnmarshallOssJson

Checklist

  • Tests added and all succeed
  • Regenerated mocks, etc. (make generate)
  • Linted (make lint-fix)
  • README.md updated, if user-facing
  • License file updated, if new 3rd-party dependency is introduced

PR Type

Enhancement, Tests


Description

  • Defer default environment readiness checks to scanner.

  • Optimize hot-path allocations and map pre-sizing.

  • Refine OSS JSON parsing and error reporting.

  • Introduce new benchmarks and tests.


File Walkthrough

Relevant files
Performance
4 files
service.go
Reduce hover channel buffer size                                                 
+1/-1     
scan_state_aggregator.go
Optimize scan state aggregation and caching                           
+10/-5   
converter.go
Optimize OSS JSON unmarshalling and error reporting           
+6/-5     
config_resolver.go
Optimize config resolver with locked getter                           
+16/-11 
Tests
7 files
scan_state_aggregator_bench_test.go
Add StateSnapshot benchmark                                                           
+109/-0 
scanner_test.go
Add tests for scanner env readiness handling                         
+93/-0   
cli_extension_executor_test.go
Update ExtensionExecutor env readiness test                           
+4/-48   
cli_test.go
Update CLI getCommand env readiness test                                 
+4/-49   
converter_bench_test.go
Add UnmarshallOssJson benchmark                                                   
+37/-0   
oss_test.go
Update OSS tests for refactored issue conversion                 
+5/-9     
config_resolver_bench_test.go
Add config resolver benchmarks                                                     
+65/-0   
Refactoring
6 files
scanner.go
Defer default env check to scanner pre-scan                           
+24/-9   
cli.go
Remove default env wait from CLI getCommand                           
+0/-5     
cli_extension_executor.go
Remove default env wait from ExtensionExecutor                     
+0/-4     
issue.go
Refactor OSS issue conversion logic                                           
+17/-16 
types.go
Simplify GetExtendedMessage signature                                       
+2/-2     
unified_converter.go
Align unified converter with GetExtendedMessage signature
+5/-2     

Consolidate config resolution and scan-state work, tighten OSS conversion,
trim hover channel buffer, and wait for default env only in the delegating
scanner so CLI initialization is not blocked.

- ConfigResolver: single RLock and getBoolLocked for DisplayableIssueTypesForFolder
- ScanStateAggregator: per-folder issue-type cache and pre-sized maps
- OSS: byte prefix for JSON array detection; vulnerability index by ID;
  GetExtendedMessage takes resolved format string (fewer config lookups per issue)
- Hover: reduce hover channel buffer from 10000 to 100
- Scanner: add preScanChecks; move WaitForDefaultEnv from CLI helpers to Scan
- Benchmarks for config resolver, state snapshot, and UnmarshallOssJson

UnmarshallOssJson errors now report input length instead of embedding the full
payload in the error string.
@bastiandoetsch
Copy link
Copy Markdown
Collaborator Author

/describe

@snyk-io
Copy link
Copy Markdown

snyk-io bot commented Apr 14, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@snyk-pr-review-bot
Copy link
Copy Markdown

PR Description updated to latest commit (d159d9e)

@bastiandoetsch bastiandoetsch marked this pull request as ready for review April 14, 2026 07:54
@bastiandoetsch bastiandoetsch requested review from a team as code owners April 14, 2026 07:54
@snyk-pr-review-bot

This comment has been minimized.

Address PR review feedback: direct byte check res[0]=='[' could fail
if CLI output contains leading whitespace or BOM. Use bytes.TrimLeft
to strip whitespace before the check while remaining allocation-free.
@snyk-pr-review-bot

This comment has been minimized.

// so the tree builder can distinguish "not yet scanned" from "scan completed with 0 issues".
func (agg *ScanStateAggregator) productScanStates(stateMap scanStateMap) map[types.FilePath]map[product.Product]bool {
states := make(map[types.FilePath]map[product.Product]bool)
states := make(map[types.FilePath]map[product.Product]bool, len(stateMap)/4+1)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why /4+1 ?

@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Observability Regression 🟠 [major]

In UnmarshallOssJson, the error reporting for failed JSON unmarshalling now only includes the input length rather than the actual payload. While this reduces memory usage, it severely degrades the ability for support and developers to debug CLI failures (e.g., when a proxy returns an HTML error page or the CLI prints a non-JSON error to stdout). The existing code used errors.Join to include the full output string, which was vital for diagnosing network and environment issues.

		err = errors.Join(err, fmt.Errorf("couldn't unmarshal CLI response. Input length: %d", len(res)))
		return nil, err
	}
} else {
	var result scanResult
	err = json.Unmarshal(res, &result)
	if err != nil {
		err = errors.Join(err, fmt.Errorf("couldn't unmarshal CLI response. Input length: %d", len(res)))
		return nil, err
Performance Bottleneck 🟡 [minor]

Reducing the hoverChan buffer size from 10,000 to 100 may cause the entire scan pipeline to stall during large workspace scans. In DelegatingConcurrentScanner.Scan, product scanners run in parallel and call processResults (which eventually produces hovers). If the listener in createHoverListener is slower than the scanner burst (e.g., processing 4 products across 50 folders simultaneously), the scanner goroutines will block until the channel is drained, artificially increasing total scan time.

s.hoverChan = make(chan DocumentHovers, 100)
📚 Repository Context Analyzed

This review considered 46 relevant code sections from 15 files (average relevance: 0.91)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants