WIP: Speedup search, part 3: don't allocate while searching for attributes #1687

Draft
fyrchik wants to merge 2 commits from fyrchik/frostfs-node:speedup-search-3 into master
Owner

This PR implements (3) suggestion from #1685.
Please, review the second commit only.
There is a noticeable improvement, but I don't want our metabase to become new tree service.
I have tried to compress the code as much as possible and make it easy to validate.
The main purpose of this PR is to discuss whether we want to go for such optimizations at all.

This PR depends on #1686.
Comparison with #1686:

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
                          │     1686     │              zeroalloc               │
                          │    sec/op    │    sec/op     vs base                │
Select/string_equal-8       3.473m ± 13%   1.469m ± 12%  -57.69% (p=0.000 n=10)
Select/string_not_equal-8   3.287m ±  5%   1.433m ±  7%  -56.41% (p=0.000 n=10)
Select/common_prefix-8      3.274m ±  7%   1.343m ±  5%  -58.98% (p=0.000 n=10)
Select/unknown-8            3.223m ±  7%   1.322m ±  5%  -58.98% (p=0.000 n=10)
geomean                     3.313m         1.391m        -58.03%

                          │     1686      │              zeroalloc               │
                          │     B/op      │     B/op      vs base                │
Select/string_equal-8       1953.0Ki ± 0%   578.0Ki ± 0%  -70.40% (p=0.000 n=10)
Select/string_not_equal-8   1953.0Ki ± 0%   578.0Ki ± 0%  -70.40% (p=0.000 n=10)
Select/common_prefix-8      1953.0Ki ± 0%   578.0Ki ± 0%  -70.41% (p=0.000 n=10)
Select/unknown-8            1945.2Ki ± 0%   570.2Ki ± 0%  -70.68% (p=0.000 n=10)
geomean                      1.905Mi        576.0Ki       -70.48%

                          │    1686     │              zeroalloc              │
                          │  allocs/op  │  allocs/op   vs base                │
Select/string_equal-8       47.03k ± 0%   13.04k ± 0%  -72.27% (p=0.000 n=10)
Select/string_not_equal-8   47.03k ± 0%   13.04k ± 0%  -72.27% (p=0.000 n=10)
Select/common_prefix-8      47.03k ± 0%   13.04k ± 0%  -72.27% (p=0.000 n=10)
Select/unknown-8            46.04k ± 0%   12.05k ± 0%  -73.82% (p=0.000 n=10)
geomean                     46.78k        12.79k       -72.67%
This PR implements (3) suggestion from #1685. Please, review the second commit only. There is a noticeable improvement, but I don't want our metabase to become new tree service. I have tried to compress the code as much as possible and make it easy to validate. The main purpose of this PR is to discuss whether we want to go for such optimizations at all. This PR depends on #1686. Comparison with #1686: ``` 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 │ 1686 │ zeroalloc │ │ sec/op │ sec/op vs base │ Select/string_equal-8 3.473m ± 13% 1.469m ± 12% -57.69% (p=0.000 n=10) Select/string_not_equal-8 3.287m ± 5% 1.433m ± 7% -56.41% (p=0.000 n=10) Select/common_prefix-8 3.274m ± 7% 1.343m ± 5% -58.98% (p=0.000 n=10) Select/unknown-8 3.223m ± 7% 1.322m ± 5% -58.98% (p=0.000 n=10) geomean 3.313m 1.391m -58.03% │ 1686 │ zeroalloc │ │ B/op │ B/op vs base │ Select/string_equal-8 1953.0Ki ± 0% 578.0Ki ± 0% -70.40% (p=0.000 n=10) Select/string_not_equal-8 1953.0Ki ± 0% 578.0Ki ± 0% -70.40% (p=0.000 n=10) Select/common_prefix-8 1953.0Ki ± 0% 578.0Ki ± 0% -70.41% (p=0.000 n=10) Select/unknown-8 1945.2Ki ± 0% 570.2Ki ± 0% -70.68% (p=0.000 n=10) geomean 1.905Mi 576.0Ki -70.48% │ 1686 │ zeroalloc │ │ allocs/op │ allocs/op vs base │ Select/string_equal-8 47.03k ± 0% 13.04k ± 0% -72.27% (p=0.000 n=10) Select/string_not_equal-8 47.03k ± 0% 13.04k ± 0% -72.27% (p=0.000 n=10) Select/common_prefix-8 47.03k ± 0% 13.04k ± 0% -72.27% (p=0.000 n=10) Select/unknown-8 46.04k ± 0% 12.05k ± 0% -73.82% (p=0.000 n=10) geomean 46.78k 12.79k -72.67% ```
fyrchik added 2 commits 2025-03-19 10:48:35 +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>
WIP: metabase: Find attribute without full unmarshal
Some checks failed
DCO action / DCO (pull_request) Failing after 30s
Tests and linters / Staticcheck (pull_request) Failing after 1m16s
Vulncheck / Vulncheck (pull_request) Successful in 1m12s
Tests and linters / Lint (pull_request) Failing after 1m26s
Pre-commit hooks / Pre-commit (pull_request) Successful in 1m33s
Build / Build Components (pull_request) Successful in 1m41s
Tests and linters / Run gofumpt (pull_request) Successful in 3m38s
Tests and linters / Tests (pull_request) Successful in 4m25s
Tests and linters / gopls check (pull_request) Successful in 5m55s
Tests and linters / Tests with -race (pull_request) Successful in 6m9s
aac2449d49
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
requested reviews from storage-core-committers, storage-core-developers 2025-03-19 10:48:35 +00:00
fyrchik reviewed 2025-03-19 10:49:33 +00:00
@ -136,0 +135,4 @@
if err != nil {
return nil, err
}
return par.Marshal()
Author
Owner

This is bad, it will be fixed, of course.

This is bad, it will be fixed, of course.
Author
Owner

We can do much better


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
BenchmarkParentFromChild/unoptimized-8         	  369690	      4090 ns/op	    3232 B/op	      59 allocs/op
BenchmarkParentFromChild/proto_access-8        	 4167025	       320.7 ns/op	     432 B/op	       2 allocs/op
PASS
coverage: 1.4% of statements

What bothers me is that this new code should also do marshaling, because we Parent() method on child reassembles object from fields in its split header.
On the other hand, we only need header in SEARCH, so we might avoid marshaling at the cost of making getRaw more complex.

We can do much better ``` 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 BenchmarkParentFromChild/unoptimized-8 369690 4090 ns/op 3232 B/op 59 allocs/op BenchmarkParentFromChild/proto_access-8 4167025 320.7 ns/op 432 B/op 2 allocs/op PASS coverage: 1.4% of statements ``` What bothers me is that this new code should also do marshaling, because we `Parent()` method on child reassembles object from fields in its split header. On the other hand, we only need header in SEARCH, so we might avoid marshaling at the cost of making `getRaw` more complex.
fyrchik changed title from Speedup search, part 3: don't allocate while searching for attributes to WIP: Speedup search, part 3: don't allocate while searching for attributes 2025-03-19 10:49:42 +00:00
fyrchik added the
discussion
refactoring
perfomance
internal
labels 2025-03-19 10:55:05 +00:00
fyrchik force-pushed speedup-search-3 from aac2449d49 to 1ec5377241 2025-03-20 13:15:19 +00:00 Compare
Some checks failed
DCO action / DCO (pull_request) Failing after 32s
Required
Details
Vulncheck / Vulncheck (pull_request) Successful in 1m10s
Required
Details
Pre-commit hooks / Pre-commit (pull_request) Successful in 1m31s
Required
Details
Tests and linters / Lint (pull_request) Failing after 1m59s
Required
Details
Tests and linters / Staticcheck (pull_request) Failing after 1m55s
Required
Details
Tests and linters / Tests (pull_request) Failing after 1m57s
Required
Details
Build / Build Components (pull_request) Successful in 2m13s
Required
Details
Tests and linters / Tests with -race (pull_request) Failing after 3m25s
Required
Details
Tests and linters / gopls check (pull_request) Successful in 3m44s
Required
Details
Tests and linters / Run gofumpt (pull_request) Successful in 4m37s
Required
Details
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u speedup-search-3:fyrchik-speedup-search-3
git checkout fyrchik-speedup-search-3
Sign in to join this conversation.
No reviewers
TrueCloudLab/storage-core-committers
TrueCloudLab/storage-core-developers
No milestone
No project
No assignees
1 participant
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#1687
No description provided.