TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 109 Pull requests 3 Projects Releases 2 Activity Actions

Optimize complex object assembly #539

Merged
fyrchik merged 2 commits from dstepanov-yadro/frostfs-node:fix/assembling_optimizations into master 2024-09-04 19:51:02 +00:00
Conversation 0 Commits 2 Files changed 3 +26 -14
3 changed files with 26 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
pkg/services/object/get/assemble.go
View file

@ -113,8 +113,10 @@ func (r *request) HeadObject(ctx context.Context, id oid.ID) (*objectSDK.Object,
return w.Object(), nil
}
func (r *request) GetObject(ctx context.Context, id oid.ID, rng *objectSDK.Range) (*objectSDK.Object, error) {
w := NewSimpleObjectWriter()
func (r *request) GetObjectAndWritePayload(ctx context.Context, id oid.ID, rng *objectSDK.Range, writer ChunkWriter) (*objectSDK.Object, error) {
w := &payloadWriter{
origin: writer,
}
p := r.prm
p.common = p.common.WithLocalOnly(false)
@ -127,7 +129,7 @@ func (r *request) GetObject(ctx context.Context, id oid.ID, rng *objectSDK.Range
if err := r.getObjectWithIndependentRequest(ctx, p); err != nil {
return nil, err
}
return w.Object(), nil
return w.obj, nil
}
func (r *request) getObjectWithIndependentRequest(ctx context.Context, prm RequestParameters) error {

16
pkg/services/object/get/assembler.go
View file

@ -10,7 +10,7 @@ import (
)
type objectGetter interface {
GetObject(ctx context.Context, id oid.ID, rng *objectSDK.Range) (*objectSDK.Object, error)
GetObjectAndWritePayload(ctx context.Context, id oid.ID, rng *objectSDK.Range, writer ChunkWriter) (*objectSDK.Object, error)
aarifullin commented 2023-07-27 09:04:00 +00:00
Outdated
Review
Copy link

I find the idea for the such optimization as excellent, but didn't you consider to return payload from the method instead passing the writer?

type ObjectAndFriends struct {
   object *objectSDK.Object
   pld    []byte
}

for

func (r *request) GetObjectAndWritePayload(ctx context.Context, id oid.ID, rng *objectSDK.Range, writer ChunkWriter) (ObjectAndFriends, error) {
}

I am afraid that with this hack (that works perfectly anyway) we may break SRP (I am really confused when one method gets and sends something in the same place) and dependency inversion (the point to pass an instance that implements the interface gives us the opportunity that we barely can use - we pass NewSimpleObjectWriter but will we ever wrap something else?)

Fix me, if I skipped or didn't get something!

I find the idea for the such optimization as excellent, but didn't you consider to return payload from the method instead passing the writer? ``` type ObjectAndFriends struct { object *objectSDK.Object pld []byte } ``` for ``` func (r *request) GetObjectAndWritePayload(ctx context.Context, id oid.ID, rng *objectSDK.Range, writer ChunkWriter) (ObjectAndFriends, error) { } ``` I am afraid that with this hack (that works perfectly anyway) we may break SRP (I am really confused when one method gets and sends something in the same place) and dependency inversion (the point to pass an instance that implements the interface gives us the opportunity that we barely can use - we pass `NewSimpleObjectWriter` but will we ever wrap something else?) Fix me, if I skipped or didn't get something!
dstepanov-yadro commented 2023-07-27 14:01:09 +00:00
Outdated
Review
Copy link

return payload from the method instead passing the writer?

Then we will have to allocate memory for the entire object, and I'm just trying to avoid this.

we pass NewSimpleObjectWriter but will we ever wrap something else

NewSimpleObjectWriter() passes only in one place. See assemblePayloadByObjectIDs: writer writes directly to response stream.

> return payload from the method instead passing the writer? Then we will have to allocate memory for the entire object, and I'm just trying to avoid this. > we pass NewSimpleObjectWriter but will we ever wrap something else `NewSimpleObjectWriter()` passes only in one place. See `assemblePayloadByObjectIDs`: `writer` writes directly to response stream.
👍 1
HeadObject(ctx context.Context, id oid.ID) (*objectSDK.Object, error)
}
@ -77,10 +77,12 @@ func (a *assembler) getLastPartOrLinkObjectID() (oid.ID, bool) {
}
func (a *assembler) initializeFromSourceObjectID(ctx context.Context, id oid.ID) (*oid.ID, []oid.ID, error) {
sourceObject, err := a.getChildObject(ctx, id, nil, true)
w := NewSimpleObjectWriter()
sourceObject, err := a.getChildObject(ctx, id, nil, true, w)
if err != nil {
return nil, nil, err
}
sourceObject.SetPayload(w.pld)
parentObject := sourceObject.Parent()
if parentObject == nil {
@ -131,8 +133,8 @@ func (a *assembler) initializeFromSourceObjectID(ctx context.Context, id oid.ID)
return nil, sourceObject.Children(), nil
}
func (a *assembler) getChildObject(ctx context.Context, id oid.ID, rng *objectSDK.Range, verifyIsChild bool) (*objectSDK.Object, error) {
obj, err := a.objGetter.GetObject(ctx, id, rng)
func (a *assembler) getChildObject(ctx context.Context, id oid.ID, rng *objectSDK.Range, verifyIsChild bool, writer ChunkWriter) (*objectSDK.Object, error) {
obj, err := a.objGetter.GetObjectAndWritePayload(ctx, id, rng, writer)
if err != nil {
return nil, err
}
@ -182,14 +184,10 @@ func (a *assembler) assemblePayloadByObjectIDs(ctx context.Context, writer Objec
r = &partRanges[i]
}
child, err := a.getChildObject(ctx, partIDs[i], r, verifyIsChild)
_, err := a.getChildObject(ctx, partIDs[i], r, verifyIsChild, writer)
if err != nil {
return err
}
if err := writer.WriteChunk(ctx, child.Payload()); err != nil {
return err
}
}
return nil
}

16
pkg/services/object/get/writer.go
View file

@ -51,9 +51,7 @@ func NewSimpleObjectWriter() *SimpleObjectWriter {
func (s *SimpleObjectWriter) WriteHeader(_ context.Context, obj *objectSDK.Object) error {
s.obj = obj
fyrchik commented 2023-07-24 13:17:33 +00:00
Outdated
Review
Copy link

argh

argh
s.pld = make([]byte, 0, obj.PayloadSize())
return nil
}
@ -82,3 +80,17 @@ func (h *hasherWrapper) WriteChunk(_ context.Context, p []byte) error {
_, err := h.hash.Write(p)
return err
}
type payloadWriter struct {
origin ChunkWriter
obj *objectSDK.Object
}
func (w *payloadWriter) WriteChunk(ctx context.Context, p []byte) error {
return w.origin.WriteChunk(ctx, p)
}
func (w *payloadWriter) WriteHeader(_ context.Context, o *objectSDK.Object) error {
w.obj = o
return nil
}