Skip to content

fix(castore): introduce fast path while getting torrent metadata to reduce allocs#611

Open
sambhav-jain-16 wants to merge 6 commits into
uber:masterfrom
sambhav-jain-16:torrent-metadata-alloc
Open

fix(castore): introduce fast path while getting torrent metadata to reduce allocs#611
sambhav-jain-16 wants to merge 6 commits into
uber:masterfrom
sambhav-jain-16:torrent-metadata-alloc

Conversation

@sambhav-jain-16
Copy link
Copy Markdown
Collaborator

@sambhav-jain-16 sambhav-jain-16 commented May 2, 2026

What?

TorrentMeta.Serialize/Deserialize round-trip svia JSON (json.Marshal + json.Unmarshal) for a MetaInfo. For every blob this allocates a fresh []byte buffer and a new *core.MetaInfo on every call to GetCacheFileMetadata. The fast path short-circuits it when the entry is already in the memory cache it hands back the existing *core.MetaInfo pointer directly, so a metadata read becomes a single pointer assignment with zero heap allocations.

Why only TorrentMetadata ?

TorrentMeta is the only metadata type whose backing value (*core.MetaInfo) is immutable after creation. Other metadata types (e.g. LastAccessTime) are mutable values written on every access.

Testing

Added a benchmark BenchmarkGetCacheFileMetadata_MemoryCache with different blob sizes and metadata sizes
Following is the benchstat comparison.

goos: linux
goarch: amd64
pkg: github.com/uber/kraken/lib/store
cpu: AMD EPYC 7B13
                                                       │ /home/user/kraken/bench-results/run-20260502-161433-n50/before.txt │ /home/user/kraken/bench-results/run-20260502-161433-n50/after.txt │
                                                       │                               sec/op                               │                  sec/op                    vs base                │
GetCacheFileMetadata_MemoryCache/1MB_4pc-96                                                                   7503.00n ± 1%                                 57.97n ± 1%  -99.23% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_64pc-96                                                                27221.50n ± 1%                                 56.61n ± 1%  -99.79% (n=50)
GetCacheFileMetadata_MemoryCache/64MB_256pc-96                                                               89182.50n ± 1%                                 57.67n ± 1%  -99.94% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_4pc_4MBpc-96                                                            7256.50n ± 1%                                 56.99n ± 3%  -99.21% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_16pc_1MBpc-96                                                          11599.50n ± 1%                                 61.23n ± 2%  -99.47% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_1024pc_16KBpc-96                                                      334380.00n ± 2%                                 61.69n ± 4%  -99.98% (p=0.000 n=50)
geomean                                                                                                         28.29µ                                      58.66n       -99.79%

                                                       │ /home/user/kraken/bench-results/run-20260502-161433-n50/before.txt │ /home/user/kraken/bench-results/run-20260502-161433-n50/after.txt │
                                                       │                                B/op                                │                       B/op                         vs base        │
GetCacheFileMetadata_MemoryCache/1MB_4pc-96                                                                   1907.000 ± 0%                                          8.000 ± 0%  -99.58% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_64pc-96                                                                 5103.000 ± 0%                                          8.000 ± 0%  -99.84% (n=50)
GetCacheFileMetadata_MemoryCache/64MB_256pc-96                                                               16494.500 ± 0%                                          8.000 ± 0%  -99.95% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_4pc_4MBpc-96                                                            1898.000 ± 0%                                          8.000 ± 0%  -99.58% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_16pc_1MBpc-96                                                           2732.000 ± 0%                                          8.000 ± 0%  -99.71% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_1024pc_16KBpc-96                                                       67460.500 ± 0%                                          8.000 ± 0%  -99.99% (n=50)
geomean                                                                                                        6.043Ki                                               8.000       -99.87%

                                                       │ /home/user/kraken/bench-results/run-20260502-161433-n50/before.txt │ /home/user/kraken/bench-results/run-20260502-161433-n50/after.txt │
                                                       │                             allocs/op                              │                     allocs/op                      vs base        │
GetCacheFileMetadata_MemoryCache/1MB_4pc-96                                                                     37.000 ± 0%                                          1.000 ± 0%  -97.30% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_64pc-96                                                                  103.000 ± 0%                                          1.000 ± 0%  -99.03% (n=50)
GetCacheFileMetadata_MemoryCache/64MB_256pc-96                                                                 299.000 ± 0%                                          1.000 ± 0%  -99.67% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_4pc_4MBpc-96                                                              37.000 ± 0%                                          1.000 ± 0%  -97.30% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_16pc_1MBpc-96                                                             52.000 ± 0%                                          1.000 ± 0%  -98.08% (n=50)
GetCacheFileMetadata_MemoryCache/16MB_1024pc_16KBpc-96                                                        1072.000 ± 0%                                          1.000 ± 0%  -99.91% (n=50)
geomean                                                                                                          115.3                                               1.000       -99.13%

Copilot AI review requested due to automatic review settings May 2, 2026 15:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves CAStore.GetCacheFileMetadata performance when serving torrent metadata from the in-memory cache by avoiding per-read serialization/deserialization, and adds coverage/benchmarks to validate allocation behavior.

Changes:

  • Add a fast path in CAStore.GetCacheFileMetadata to return the cached *core.MetaInfo pointer directly for *metadata.TorrentMeta.
  • Add a unit test asserting the no-copy behavior when reading metadata from the memory cache.
  • Add a benchmark suite to measure allocations/time across different blob sizes and piece counts for memory-cache metadata reads.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/store/ca_store.go Returns cached *core.MetaInfo directly for torrent metadata reads from memory cache, avoiding JSON round-trips.
lib/store/ca_store_test.go Adds a no-copy unit test and a benchmark measuring metadata read allocations across scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/store/ca_store_test.go
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/store/ca_store_test.go
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sambhav-jain-16 sambhav-jain-16 changed the title Torrent metadata alloc fix(castore): introduce fast patch while getting torrent metadata to reduce allocs May 3, 2026
@sambhav-jain-16 sambhav-jain-16 marked this pull request as ready for review May 3, 2026 10:23
@sambhav-jain-16 sambhav-jain-16 changed the title fix(castore): introduce fast patch while getting torrent metadata to reduce allocs fix(castore): introduce fast path while getting torrent metadata to reduce allocs May 3, 2026
Copy link
Copy Markdown
Collaborator

@thijmv thijmv left a comment

Choose a reason for hiding this comment

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

A couple of comments:

Comment thread lib/store/ca_store.go Outdated
Comment thread lib/store/ca_store_test.go Outdated
Comment thread lib/store/ca_store_test.go Outdated
Copilot AI review requested due to automatic review settings May 8, 2026 17:24
@sambhav-jain-16 sambhav-jain-16 force-pushed the torrent-metadata-alloc branch from 6aaaf7d to 33637e0 Compare May 8, 2026 17:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread lib/store/ca_store.go
Comment on lines +483 to +485
// hand back cached pointer
tm.MetaInfo = entry.MetaInfo
return nil
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

?

Comment thread lib/store/ca_store.go Outdated
Comment thread lib/store/ca_store.go Outdated
Comment thread lib/store/ca_store.go Outdated
Comment thread lib/store/ca_store_test.go Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants