nspcc-dev / neofs-node

NeoFS is a decentralized distributed object storage integrated with the Neo blockchain
https://fs.neo.org
GNU General Public License v3.0
32 stars 38 forks source link

engine: Support reading objects without decoding #2753

Closed cthulhu-rider closed 7 months ago

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 79.05759% with 40 lines in your changes are missing coverage. Please review.

Project coverage is 22.03%. Comparing base (2da3676) to head (09e385c).

Files Patch % Lines
pkg/local_object_storage/blobstor/fstree/fstree.go 44.44% 11 Missing and 4 partials :warning:
pkg/local_object_storage/blobstor/peapod/peapod.go 66.66% 6 Missing and 1 partial :warning:
cmd/blobovnicza-to-peapod/blobovniczatree/get.go 0.00% 5 Missing :warning:
...al_object_storage/blobstor/compression/compress.go 0.00% 5 Missing :warning:
pkg/local_object_storage/engine/get.go 89.18% 2 Missing and 2 partials :warning:
pkg/local_object_storage/shard/get.go 95.34% 2 Missing :warning:
pkg/local_object_storage/writecache/get.go 87.50% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2753 +/- ## ========================================== + Coverage 21.89% 22.03% +0.14% ========================================== Files 787 787 Lines 46643 46763 +120 ========================================== + Hits 10211 10304 +93 - Misses 35555 35576 +21 - Partials 877 883 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cthulhu-rider commented 7 months ago

Not clear why we need it

tried to explain in the commit. This is 100%-needed functionality for any storage IMO - just get what u store as []byte

cthulhu-rider commented 7 months ago

increased test coverage. Fixed old bug on the way

roman-khimov commented 7 months ago

We need to have some broader context for this. It looks like a beginning of a custom memory allocator which just shouldn't be done. We can mmap things, we can stream things, we can pool some chunks (but it doesn't work well for objects of different sizes unfortunately). If nothing else works in-place allocation is an option too (which we had for years anyway, just in a bit different form), especially given that it will be more effective already (one flat buffer, no decoding).

cthulhu-rider commented 7 months ago

ok lets use make only for now

cthulhu-rider commented 7 months ago

dropped options

cthulhu-rider commented 7 months ago

we need object streaming RPC for a correct optimization

two different topics. Here we cover redundant decoding, while ur talking about smaller bufferization (which is also good to be done, but unrelated to the current PR/issue)

carpawell commented 7 months ago

two different topics

Not different to me, the main idea is the same.

ur talking about smaller bufferization

Im talking about less memory consumption and/or allocations number. RPC that streams an object with partial reading may improve important things by orders of magnitude, while reading the whole object into memory and transferring it as a single message should only make it a few times better.