Skip to content

Conversation

@kaushik-rohit
Copy link
Contributor

#108

Building on previous PR #175, I updated the code to be standard Go benchmarking and addressed the comments in that PR.

Profiling run:

goos: darwin
goarch: arm64
pkg: github.com/llm-d/llm-d-kv-cache/tests/profiling/kv_cache_index
cpu: Apple M3 Pro
BenchmarkInMemory_Add-12        	     192	   6086106 ns/op
BenchmarkInMemory_Lookup-12     	       6	 178765021 ns/op
BenchmarkRedis_Add-12           	       4	 252224448 ns/op
BenchmarkRedis_Lookup-12        	       7	 143262946 ns/op
BenchmarkCostAware_Add-12       	     128	  11420663 ns/op
BenchmarkCostAware_Lookup-12    	       3	 440866472 ns/op
PASS
ok  	github.com/llm-d/llm-d-kv-cache/tests/profiling/kv_cache_index	13.403s

- Replaced standalone main.go with index_benchmark_test.go for better CI integration.
- Implemented fixed random seed to ensure identical workloads across all index implementations.
- Added 'miniredis' integration to make Redis benchmarks self-contained.
- Updated 'index.Add' usage to match the current interface signature.
- Added clarifying comments regarding empty pod set lookups.
@yankay
Copy link
Collaborator

yankay commented Jan 12, 2026

HI @kaushik-rohit

Would it be better to add a README to describe how to run it?

@vMaroon vMaroon requested a review from sagearc January 13, 2026 00:18
@kaushik-rohit
Copy link
Contributor Author

@yankay That makes sense, I added a README

@yankay
Copy link
Collaborator

yankay commented Jan 15, 2026

@yankay That makes sense, I added a README

Thanks @kaushik-rohit

It's a great change :-)
/lgtm

The test result on my PC:

(.venv) root@gpu-3090:~/oss/llm-d-kv-cache/tests/profiling/kv_cache_index# go test -bench=. -benchmem
goos: linux
goarch: amd64
pkg: github.com/llm-d/llm-d-kv-cache/tests/profiling/kv_cache_index
cpu: AMD Ryzen 9 3900 12-Core Processor
BenchmarkInMemory_Add-24                      45          23630930 ns/op        13737528 B/op     130167 allocs/op
BenchmarkInMemory_Lookup-24                    2         615924412 ns/op        2564472368 B/op   131676 allocs/op
BenchmarkRedis_Add-24                          3         375040929 ns/op        21154280 B/op     556949 allocs/op
BenchmarkRedis_Lookup-24                       7         185071392 ns/op         9064401 B/op     260133 allocs/op
BenchmarkCostAware_Add-24                     26          43422847 ns/op        11363554 B/op     132215 allocs/op
BenchmarkCostAware_Lookup-24                   1        1131001154 ns/op        2564395856 B/op   160103 allocs/op
PASS

@yankay
Copy link
Collaborator

yankay commented Jan 19, 2026

HI @sagearc @hyeongyun0916 @liu-cong @vMaroon

Would you please help to review it :-)

@vMaroon
Copy link
Member

vMaroon commented Jan 19, 2026

Thanks @kaushik-rohit, @yankay, apologies for delay.

/lgtm
/approve

@github-actions github-actions bot added the lgtm label Jan 19, 2026
@github-actions github-actions bot merged commit e2a0a64 into llm-d:main Jan 19, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants