From 044c3733e92aaee4bef5eea9ae5c28137fbc6158 Mon Sep 17 00:00:00 2001 From: Robert Vasek Date: Thu, 10 Feb 2022 20:50:44 +0100 Subject: [PATCH] rados: use rados_write_op_omap_set2() WriteOp.SetOmap now uses rados_write_op_omap_set2(). The entirety of setOmapStep struct was removed as it's not necessary for it to own the various C-allocated values passed to rados_write_op_omap_set2() as arguments. rados_write_op_omap_set2() makes copies of all the string-based args [1], so it's sufficient for them to be short lived. [1] https://github.com/ceph/ceph/blob/2bd3dd512ab42f6e5d5d0dd5f5428b783681802e/src/librados/librados_c.cc#L3852-L3871 --- rados/omap.go | 61 ----------------------------------------------- rados/write_op.go | 28 ++++++++++++++++------ 2 files changed, 21 insertions(+), 68 deletions(-) diff --git a/rados/omap.go b/rados/omap.go index 4de9c73..f89babd 100644 --- a/rados/omap.go +++ b/rados/omap.go @@ -10,69 +10,8 @@ import "C" import ( "runtime" "unsafe" - - "github.com/ceph/go-ceph/internal/cutil" ) -// setOmapStep is a write op step. It holds C memory used in the operation. -type setOmapStep struct { - withRefs - withoutUpdate - - // C arguments - cKeys cutil.CPtrCSlice - cValues cutil.CPtrCSlice - cLengths cutil.SizeTCSlice - cNum C.size_t -} - -func newSetOmapStep(pairs map[string][]byte) *setOmapStep { - - maplen := len(pairs) - cKeys := cutil.NewCPtrCSlice(maplen) - cValues := cutil.NewCPtrCSlice(maplen) - cLengths := cutil.NewSizeTCSlice(maplen) - - sos := &setOmapStep{ - cKeys: cKeys, - cValues: cValues, - cLengths: cLengths, - cNum: C.size_t(maplen), - } - - var i uintptr - for key, value := range pairs { - // key - ck := C.CString(key) - sos.add(unsafe.Pointer(ck)) - cKeys[i] = cutil.CPtr(ck) - - // value and its length - vlen := cutil.SizeT(len(value)) - if vlen > 0 { - cv := C.CBytes(value) - sos.add(cv) - cValues[i] = cutil.CPtr(cv) - } else { - cValues[i] = nil - } - - cLengths[i] = vlen - - i++ - } - - runtime.SetFinalizer(sos, opStepFinalizer) - return sos -} - -func (sos *setOmapStep) free() { - sos.cKeys.Free() - sos.cValues.Free() - sos.cLengths.Free() - sos.withRefs.free() -} - // OmapKeyValue items are returned by the GetOmapStep's Next call. type OmapKeyValue struct { Key string diff --git a/rados/write_op.go b/rados/write_op.go index cdb838d..759d9e0 100644 --- a/rados/write_op.go +++ b/rados/write_op.go @@ -10,6 +10,7 @@ import "C" import ( "unsafe" + "github.com/ceph/go-ceph/internal/cutil" ts "github.com/ceph/go-ceph/internal/timespec" ) @@ -92,14 +93,27 @@ func (w *WriteOp) Create(exclusive CreateOption) { // SetOmap appends the map `pairs` to the omap `oid`. func (w *WriteOp) SetOmap(pairs map[string][]byte) { - sos := newSetOmapStep(pairs) - w.steps = append(w.steps, sos) - C.rados_write_op_omap_set( + keys := make([]string, len(pairs)) + values := make([][]byte, len(pairs)) + idx := 0 + for k, v := range pairs { + keys[idx] = k + values[idx] = v + idx++ + } + + cKeys := cutil.NewBufferGroupStrings(keys) + cValues := cutil.NewBufferGroupBytes(values) + defer cKeys.Free() + defer cValues.Free() + + C.rados_write_op_omap_set2( w.op, - (**C.char)(sos.cKeys.Ptr()), - (**C.char)(sos.cValues.Ptr()), - (*C.size_t)(sos.cLengths.Ptr()), - sos.cNum) + (**C.char)(cKeys.BuffersPtr()), + (**C.char)(cValues.BuffersPtr()), + (*C.size_t)(cKeys.LengthsPtr()), + (*C.size_t)(cValues.LengthsPtr()), + (C.size_t)(len(pairs))) } // RmOmapKeys removes the specified `keys` from the omap `oid`.