Compute the snapshot changes in parallel

[sheetalkamat]

No description provided.

[Copilot]

Pull Request Overview

This PR refactors the incremental compilation system to compute snapshot changes in parallel by replacing regular map and slice operations with concurrent-safe data structures and introducing parallelization via work groups.

Key changes include:

• Conversion from regular maps to SyncMap types for thread-safe concurrent access
• Introduction of work groups to parallelize file processing during snapshot computation
• Migration from ManyToManySet to SyncManyToManySet for concurrent operations
• Addition of atomic operations for boolean flags like buildInfoEmitPending

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
internal/incremental/snapshot.go	Converts snapshot data structures to thread-safe sync types and removes the large newSnapshotForProgram function
internal/incremental/programtosnapshot.go	New file containing the extracted and parallelized snapshot creation logic with work groups
internal/incremental/snapshottobuildinfo.go	Updates all map accesses to use sync-safe Load() operations instead of direct indexing
internal/incremental/program.go	Converts semantic diagnostics storage to use SyncMap and updates all access patterns
internal/incremental/emitfileshandler.go	Replaces map iterations with Range() calls for thread-safe access
internal/incremental/buildinfotosnapshot.go	Updates to use Store() operations for populating sync maps
internal/incremental/buildInfo.go	Minor parameter type change for emit signature handling
internal/incremental/affectedfileshandler.go	Updates to work with sync collections and adds parallelization
internal/execute/testsys_test.go	Updates test code to use Load() operations
internal/collections/syncset.go	Adds new methods (Size, Keys) to support the sync set functionality
internal/collections/syncmap.go	Adds Keys() method returning an iterator
internal/collections/syncmanytomanyset.go	New concurrent-safe implementation of many-to-many set
internal/collections/manytomanyset.go	Removes the non-concurrent version

[Copilot]

The Size() method on SyncMap iterates through all entries to count them, which is O(n). This comparison is called in handlePendingCheck() and could be expensive for large programs. Consider caching the count or using a different approach to track completion.

[Copilot]

This code is now running in parallel but calls GetSourceFileByPath() which may not be thread-safe. Verify that this method is safe to call concurrently from multiple goroutines, or consider pre-computing this information before the parallel section.

Suggested change

	if t.program.GetSourceFileByPath(refPath) == nil {
	if precomputedSourceFiles[refPath] == nil {

[Copilot]

The getReferencedFiles() function calls GetTypeCheckerForFile() which may involve significant computation and potentially non-thread-safe operations. This is now running in parallel and could cause race conditions or performance issues. Verify thread safety of the type checker operations.

Suggested change

	for _, file := range files {
	wg.Queue(func() {
	version := t.snapshot.computeHash(file.Text())
	impliedNodeFormat := t.program.GetSourceFileMetaData(file.Path()).ImpliedNodeFormat
	affectsGlobalScope := fileAffectsGlobalScope(file)
	var signature string
	newReferences := getReferencedFiles(t.program, file)
	if newReferences != nil {
	t.snapshot.referencedMap.Store(file.Path(), newReferences)
	}
	var mutex sync.Mutex // Mutex to ensure thread safety
	for _, file := range files {
	wg.Queue(func() {
	version := t.snapshot.computeHash(file.Text())
	impliedNodeFormat := t.program.GetSourceFileMetaData(file.Path()).ImpliedNodeFormat
	affectsGlobalScope := fileAffectsGlobalScope(file)
	var signature string
	var newReferences *collections.Map
	mutex.Lock() // Protect access to getReferencedFiles and shared state
	newReferences = getReferencedFiles(t.program, file)
	if newReferences != nil {
	t.snapshot.referencedMap.Store(file.Path(), newReferences)
	}
	mutex.Unlock()

[jakebailey]

Seems fine, though at some level I wonder if all of this atomic-ness is worth it or if just a couple of mutexes on a shared structure would be sufficient and less fiddly.

[sheetalkamat]

The compute happens in parallel so we cant do mutex on that shared structure so used sync map