metabase: Cache frequently accessed singleton buckets #1686

Merged
fyrchik merged 1 commit from fyrchik/frostfs-node:speedup-search-2 into master 2025-03-20 10:17:43 +00:00
Owner

This PR implements (2) suggestion from #1685.

There are some buckets we access almost always, to check whether an
object is alive. In search we also iterate over lots of objects, and
tx.Bucket() shows itself a lot in pprof.

goos: linux
goarch: amd64
pkg: git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
                          │      1      │                  2                   │
                          │   sec/op    │    sec/op     vs base                │
Select/string_equal-8       4.753m ± 6%   3.969m ± 14%  -16.50% (p=0.000 n=10)
Select/string_not_equal-8   4.247m ± 9%   3.486m ± 11%  -17.93% (p=0.000 n=10)
Select/common_prefix-8      4.163m ± 5%   3.323m ±  5%  -20.18% (p=0.000 n=10)
Select/unknown-8            3.557m ± 3%   3.064m ±  8%  -13.85% (p=0.001 n=10)
geomean                     4.158m        3.445m        -17.15%

                          │      1       │                  2                   │
                          │     B/op     │     B/op      vs base                │
Select/string_equal-8       2.250Mi ± 0%   1.907Mi ± 0%  -15.24% (p=0.000 n=10)
Select/string_not_equal-8   2.250Mi ± 0%   1.907Mi ± 0%  -15.24% (p=0.000 n=10)
Select/common_prefix-8      2.250Mi ± 0%   1.907Mi ± 0%  -15.24% (p=0.000 n=10)
Select/unknown-8            2.243Mi ± 0%   1.900Mi ± 0%  -15.29% (p=0.000 n=10)
geomean                     2.248Mi        1.905Mi       -15.26%

                          │      1      │                  2                  │
                          │  allocs/op  │  allocs/op   vs base                │
Select/string_equal-8       56.02k ± 0%   47.03k ± 0%  -16.05% (p=0.000 n=10)
Select/string_not_equal-8   56.02k ± 0%   47.03k ± 0%  -16.05% (p=0.000 n=10)
Select/common_prefix-8      56.02k ± 0%   47.03k ± 0%  -16.05% (p=0.000 n=10)
Select/unknown-8            55.03k ± 0%   46.04k ± 0%  -16.34% (p=0.000 n=10)
geomean                     55.78k        46.78k       -16.12%

There are also buckets tied to a specific container, like expired.
I have intentionally left them out, because it provides a noticeable increase in complexity with little speed benefits.
We can do it later after other optimizations.

This PR implements (2) suggestion from #1685. There are some buckets we access almost always, to check whether an object is alive. In search we also iterate over lots of objects, and `tx.Bucket()` shows itself a lot in pprof. ``` goos: linux goarch: amd64 pkg: git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz │ 1 │ 2 │ │ sec/op │ sec/op vs base │ Select/string_equal-8 4.753m ± 6% 3.969m ± 14% -16.50% (p=0.000 n=10) Select/string_not_equal-8 4.247m ± 9% 3.486m ± 11% -17.93% (p=0.000 n=10) Select/common_prefix-8 4.163m ± 5% 3.323m ± 5% -20.18% (p=0.000 n=10) Select/unknown-8 3.557m ± 3% 3.064m ± 8% -13.85% (p=0.001 n=10) geomean 4.158m 3.445m -17.15% │ 1 │ 2 │ │ B/op │ B/op vs base │ Select/string_equal-8 2.250Mi ± 0% 1.907Mi ± 0% -15.24% (p=0.000 n=10) Select/string_not_equal-8 2.250Mi ± 0% 1.907Mi ± 0% -15.24% (p=0.000 n=10) Select/common_prefix-8 2.250Mi ± 0% 1.907Mi ± 0% -15.24% (p=0.000 n=10) Select/unknown-8 2.243Mi ± 0% 1.900Mi ± 0% -15.29% (p=0.000 n=10) geomean 2.248Mi 1.905Mi -15.26% │ 1 │ 2 │ │ allocs/op │ allocs/op vs base │ Select/string_equal-8 56.02k ± 0% 47.03k ± 0% -16.05% (p=0.000 n=10) Select/string_not_equal-8 56.02k ± 0% 47.03k ± 0% -16.05% (p=0.000 n=10) Select/common_prefix-8 56.02k ± 0% 47.03k ± 0% -16.05% (p=0.000 n=10) Select/unknown-8 55.03k ± 0% 46.04k ± 0% -16.34% (p=0.000 n=10) geomean 55.78k 46.78k -16.12% ``` There are also buckets tied to a specific container, like `expired`. I have intentionally left them out, because it provides a noticeable increase in complexity with little speed benefits. We can do it later after other optimizations.
fyrchik added 1 commit 2025-03-19 06:35:55 +00:00
[#1685] metabase: Cache frequently accessed singleton buckets
All checks were successful
DCO action / DCO (pull_request) Successful in 49s
Vulncheck / Vulncheck (pull_request) Successful in 1m7s
Pre-commit hooks / Pre-commit (pull_request) Successful in 1m32s
Build / Build Components (pull_request) Successful in 1m48s
Tests and linters / Run gofumpt (pull_request) Successful in 3m29s
Tests and linters / Lint (pull_request) Successful in 4m1s
Tests and linters / Staticcheck (pull_request) Successful in 4m5s
Tests and linters / Tests (pull_request) Successful in 4m49s
Tests and linters / Tests with -race (pull_request) Successful in 5m30s
Tests and linters / gopls check (pull_request) Successful in 5m42s
ec2871dd64
There are some buckets we access almost always, to check whether an
object is alive. In search we also iterate over lots of objects, and
`tx.Bucket()` shows itself a lot in pprof.
```
goos: linux
goarch: amd64
pkg: git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
                          │      1      │                  2                   │
                          │   sec/op    │    sec/op     vs base                │
Select/string_equal-8       4.753m ± 6%   3.969m ± 14%  -16.50% (p=0.000 n=10)
Select/string_not_equal-8   4.247m ± 9%   3.486m ± 11%  -17.93% (p=0.000 n=10)
Select/common_prefix-8      4.163m ± 5%   3.323m ±  5%  -20.18% (p=0.000 n=10)
Select/unknown-8            3.557m ± 3%   3.064m ±  8%  -13.85% (p=0.001 n=10)
geomean                     4.158m        3.445m        -17.15%

                          │      1       │                  2                   │
                          │     B/op     │     B/op      vs base                │
Select/string_equal-8       2.250Mi ± 0%   1.907Mi ± 0%  -15.24% (p=0.000 n=10)
Select/string_not_equal-8   2.250Mi ± 0%   1.907Mi ± 0%  -15.24% (p=0.000 n=10)
Select/common_prefix-8      2.250Mi ± 0%   1.907Mi ± 0%  -15.24% (p=0.000 n=10)
Select/unknown-8            2.243Mi ± 0%   1.900Mi ± 0%  -15.29% (p=0.000 n=10)
geomean                     2.248Mi        1.905Mi       -15.26%

                          │      1      │                  2                  │
                          │  allocs/op  │  allocs/op   vs base                │
Select/string_equal-8       56.02k ± 0%   47.03k ± 0%  -16.05% (p=0.000 n=10)
Select/string_not_equal-8   56.02k ± 0%   47.03k ± 0%  -16.05% (p=0.000 n=10)
Select/common_prefix-8      56.02k ± 0%   47.03k ± 0%  -16.05% (p=0.000 n=10)
Select/unknown-8            55.03k ± 0%   46.04k ± 0%  -16.34% (p=0.000 n=10)
geomean                     55.78k        46.78k       -16.12%
```

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
requested reviews from storage-core-committers, storage-core-developers 2025-03-19 06:35:55 +00:00
a-savchuk approved these changes 2025-03-19 07:10:39 +00:00
Dismissed
a-savchuk reviewed 2025-03-19 07:18:50 +00:00
@ -0,0 +14,4 @@
return &bucketCache{}
}
func lockedBucket(bc *bucketCache, tx *bbolt.Tx) *bbolt.Bucket {
Member

Maybe make this functions methods of the cache

Maybe make this functions methods of the cache
Member

Having functions is good but personally I don't like we have simple global names which can be easily shadowed by a local variable, and it forces to alter, invert (lockedBucket -> bucketLocked), abbreviate (lockedBucket -> lockedBkt) local variable names

*Having functions is good* but personally I don't like we have simple global names which can be easily shadowed by a local variable, and it forces to alter, invert (lockedBucket -> bucketLocked), abbreviate (lockedBucket -> lockedBkt) local variable names
Author
Owner

I was thinking about it: the reason I have used functions, is because I do a nil check inside.
From method calls I usually expect to panic on nil.
And (*bucketCache)(nil).lockedBucket(tx) looks unusual.
I will think about your suggestion a bit more.

I was thinking about it: the reason I have used functions, is because I do a nil check inside. From method calls I usually expect to panic on nil. And `(*bucketCache)(nil).lockedBucket(tx)` looks unusual. I will think about your suggestion a bit more.
Member

Maybe just rename function, so it has a verb in the name? Like getLockedBucket.

Maybe just rename function, so it has a verb in the name? Like `getLockedBucket`.
Author
Owner

Used getLockedBucket, similar for others.

Used `getLockedBucket`, similar for others.
a-savchuk marked this conversation as resolved
acid-ant approved these changes 2025-03-19 08:09:37 +00:00
Dismissed
dstepanov-yadro approved these changes 2025-03-19 13:12:42 +00:00
Dismissed
fyrchik force-pushed speedup-search-2 from ec2871dd64 to 1e89684ac2 2025-03-20 09:13:56 +00:00 Compare
fyrchik dismissed a-savchuk's review 2025-03-20 09:13:56 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

fyrchik dismissed acid-ant's review 2025-03-20 09:13:56 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

fyrchik dismissed dstepanov-yadro's review 2025-03-20 09:13:56 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

requested reviews from storage-core-committers, storage-core-developers 2025-03-20 09:18:50 +00:00
fyrchik added the
perfomance
internal
frostfs-node
labels 2025-03-20 09:23:08 +00:00
a-savchuk approved these changes 2025-03-20 10:13:47 +00:00
fyrchik merged commit a49f0717b3 into master 2025-03-20 10:17:43 +00:00
fyrchik deleted branch speedup-search-2 2025-03-20 10:17:48 +00:00
Sign in to join this conversation.
No reviewers
TrueCloudLab/storage-core-committers
No milestone
No project
No assignees
5 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#1686
No description provided.