⚡ Bolt: Pre-parse IP addresses before sorting discovery sweep results

[blue4209211]

Description

⚡ Bolt Performance Optimization

Pre-parsed string IP addresses into netip.Addr structs once into a slice before sorting discovery sweep results in [pkg/proxy/discovery/sweep.go].

💡 What

Updated runSweep to pre-parse string IP addresses once into a wrapper slice prior to sorting, rather than repeatedly calling netip.ParseAddr inside sort.Slice.

🎯 Why

In runSweep, discovered hosts are sorted by IP address before being returned. Previously, compareIPs was invoked directly inside sort.Slice, causing netip.ParseAddr to be executed on every single comparison in the O(N log N) sorting loop. For large sweeps (up to 65,536 hosts), this resulted in over 2,000,000 redundant string parsing calls and excessive allocations during result presentation.

📊 Impact

• Reduces string IP parses by >95%: String IP parsing drops from O(N log N) to O(N) (from >2M calls down to 65.5k calls for maximum scope).
• Faster sorting: Direct netip.Addr.Less comparison inside the sort loop instead of parsing IP strings on each comparison.
• Lower memory overhead: Benchmark shows only 5 total allocations per sorting operation for 1,000 hosts (171 µs/op).

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Enhancement (non-breaking change which improves existing functionality)
• Refactor (non-breaking change which improves code structure)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation
• CI/CD

How Has This Been Tested?

• Unit tests
• Manual testing

Run the benchmark in pkg/proxy/discovery/sweep_test.go:

go test -bench=BenchmarkSweepResultSorting -benchmem ./pkg/proxy/discovery/...

Checklist

• CLA signed (the CLA bot will prompt on your first PR)
• make validate passes (fmt + lint + test)
• Docs updated if the wire shape, config surface, or proxy module behavior changed

[gemini-code-assist]

Code Review

This pull request optimizes the IP address sorting logic in the host discovery sweep by pre-parsing IP strings into netip.Addr structs before sorting, avoiding expensive string parsing within the comparator function. It also adds a performance journal entry and a benchmark to measure sorting performance. The feedback suggests further optimizing this by storing pointers to SweepHost rather than copying the entire 96-byte struct during sorting swaps, and updating the benchmark accordingly.

[gemini-code-assist]

Storing the entire SweepHost struct in sweepHostAddr causes the 96-byte struct to be copied multiple times during sort.Slice swaps. Since hosts is a map of pointers (map[string]*SweepHost), we can store the pointer *SweepHost instead. This reduces the size of sweepHostAddr from 120 bytes to 32 bytes, significantly speeding up the sorting process by avoiding expensive struct copies.

type sweepHostAddr struct {
	host *SweepHost
	addr netip.Addr
}
items := make([]sweepHostAddr, 0, len(hosts))
for _, h := range hosts {
	addr, err := netip.ParseAddr(h.IP)
	if err != nil {
		items = append(items, sweepHostAddr{host: h})
		continue
	}
	items = append(items, sweepHostAddr{host: h, addr: addr})
}
sort.Slice(items, func(i, j int) bool {
	if !items[i].addr.IsValid() || !items[j].addr.IsValid() {
		return items[i].host.IP < items[j].host.IP
	}
	return items[i].addr.Less(items[j].addr)
})

out := make([]SweepHost, len(items))
for i, item := range items {
	out[i] = *item.host
}

[gemini-code-assist]

Update the benchmark to use the pointer-based sweepHostAddr struct to match the optimized implementation and ensure accurate performance measurements.

type sweepHostAddr struct {
	host *SweepHost
	addr netip.Addr
}
items := make([]sweepHostAddr, 0, len(hosts))
for _, h := range hosts {
	addr, err := netip.ParseAddr(h.IP)
	if err != nil {
		items = append(items, sweepHostAddr{host: h})
		continue
	}
	items = append(items, sweepHostAddr{host: h, addr: addr})
}
sort.Slice(items, func(i, j int) bool {
	if !items[i].addr.IsValid() || !items[j].addr.IsValid() {
		return items[i].host.IP < items[j].host.IP
	}
	return items[i].addr.Less(items[j].addr)
})
out := make([]SweepHost, len(items))
for i, item := range items {
	out[i] = *item.host
}

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request optimizes the sorting of sweep results by pre-parsing IP addresses into netip.Addr structs once before sorting, rather than parsing them repeatedly inside the sort comparator. It also adds a benchmark to measure sorting performance. The review feedback points out that the new sort comparator violates the strict weak ordering requirement of sort.Slice when handling a mix of valid and invalid IP addresses, which can lead to undefined behavior or panics. Corrected comparator implementations are suggested for both the main code and the benchmark.

[gemini-code-assist]

The comparator function violates the strict weak ordering requirement of sort.Slice when dealing with a mix of valid and invalid IP addresses.

Specifically, if one IP is invalid and the other is valid, comparing them using their raw string values (items[i].host.IP < items[j].host.IP) can create comparison cycles (e.g., A < C < B < A), violating transitivity. This can lead to unpredictable sorting behavior or incorrect ordering.

To fix this, ensure that invalid IP addresses are consistently sorted to one end (e.g., the end of the slice) and compared using string comparison only when both are invalid.

	sort.Slice(items, func(i, j int) bool {
		iValid := items[i].addr.IsValid()
		jValid := items[j].addr.IsValid()
		if !iValid && !jValid {
			return items[i].host.IP < items[j].host.IP
		}
		if !iValid {
			return false
		}
		if !jValid {
			return true
		}
		return items[i].addr.Less(items[j].addr)
	})

[gemini-code-assist]

The comparator function in the benchmark also violates the strict weak ordering requirement when handling invalid IP addresses. It should be updated to match the corrected comparator logic used in the main implementation.

		sort.Slice(items, func(i, j int) bool {
			iValid := items[i].addr.IsValid()
			jValid := items[j].addr.IsValid()
			if !iValid && !jValid {
				return items[i].host.IP < items[j].host.IP
			}
			if !iValid {
				return false
			}
			if !jValid {
				return true
			}
			return items[i].addr.Less(items[j].addr)
		})

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request optimizes the IP address sorting logic in runSweep by pre-parsing IP strings into netip.Addr structs once before sorting, preventing redundant parses inside the comparator. It also documents this optimization in a performance journal and adds a benchmark. The review feedback suggests refactoring this inline sorting logic into a dedicated sortHosts helper function to eliminate code duplication and allow the benchmark to test the production implementation directly.

[gemini-code-assist]

Instead of inlining the complex sorting logic directly inside runSweep, we should extract it into a dedicated helper function, e.g., sortHosts(hosts map[string]*SweepHost) []SweepHost. This avoids code duplication and allows the benchmark in sweep_test.go to directly test the production implementation rather than a copy-pasted version of it.

Suggested change

	// Pre-parse IP addresses once to avoid O(N log N) string parses in sort comparator.
	// Store pointers (*SweepHost) to keep sweepHostAddr small (32 bytes instead of 120 bytes)
	// and eliminate struct copying overhead during sort.Slice swaps.
	type sweepHostAddr struct {
	host *SweepHost
	addr netip.Addr
	}
	items := make([]sweepHostAddr, 0, len(hosts))
	for _, h := range hosts {
	out = append(out, *h)
	addr, err := netip.ParseAddr(h.IP)
	if err != nil {
	items = append(items, sweepHostAddr{host: h})
	continue
	}
	items = append(items, sweepHostAddr{host: h, addr: addr})
	}
	sort.Slice(items, func(i, j int) bool {
	iValid := items[i].addr.IsValid()
	jValid := items[j].addr.IsValid()
	if !iValid && !jValid {
	return items[i].host.IP < items[j].host.IP
	}
	if !iValid {
	return false
	}
	if !jValid {
	return true
	}
	return items[i].addr.Less(items[j].addr)
	})
	out := make([]SweepHost, len(items))
	for i, item := range items {
	out[i] = *item.host
	}
	out := sortHosts(hosts)

[gemini-code-assist]

Define the helper function sortHosts here to encapsulate the pre-parsing and sorting logic. This keeps runSweep clean and allows the benchmark to reuse the exact same sorting implementation.

func sortHosts(hosts map[string]*SweepHost) []SweepHost {
	type sweepHostAddr struct {
		host *SweepHost
		addr netip.Addr
	}
	items := make([]sweepHostAddr, len(hosts))
	idx := 0
	for _, h := range hosts {
		addr, err := netip.ParseAddr(h.IP)
		if err != nil {
			items[idx] = sweepHostAddr{host: h}
		} else {
			items[idx] = sweepHostAddr{host: h, addr: addr}
		}
		idx++
	}
	sort.Slice(items, func(i, j int) bool {
		iValid := items[i].addr.IsValid()
		jValid := items[j].addr.IsValid()
		if !iValid && !jValid {
			return items[i].host.IP < items[j].host.IP
		}
		if !iValid {
			return false
		}
		if !jValid {
			return true
		}
		return items[i].addr.Less(items[j].addr)
	})

	out := make([]SweepHost, len(items))
	for i, item := range items {
		out[i] = *item.host
	}
	return out
}

[gemini-code-assist]

Now that the sorting logic is extracted into sortHosts, we can simplify the benchmark to call sortHosts directly. This ensures the benchmark always tests the actual production implementation and prevents the test code from drifting out of sync with the production code.

		_ = sortHosts(hosts)