registry: optimize object marshaling #44

fyrchik · 2023-03-23T15:19:07Z

fyrchik commented

2023-03-23 15:19:07 +00:00

Close #42

Tested with 40M objects and 15G database, from >100s to setup to <1s, I expect it to work similarly for bigger databases.

Close #42 Tested with 40M objects and 15G database, from >100s to setup to <1s, I expect it to work similarly for bigger databases.

ale64bit reviewed 2023-03-24 07:02:09 +00:00

internal/registry/obj_info_test.go Outdated

													
				@ -0,0 +103,4 @@

				func randString(n int) string {

					var sb strings.Builder

					for i := 0; i < n; i++ {

						sb.WriteRune('a' + rune(rand.Int())%('z'-'a'))

ale64bit commented

2023-03-24 07:02:09 +00:00

not that it matters, but this never generates 'z'.

fyrchik commented

2023-03-24 12:29:15 +00:00

Poster

Fixed

fyrchik marked this conversation as resolved

fyrchik force-pushed optimize-registry from 7133400b50 to 8ca052084c

2023-03-24 12:29:00 +00:00

Compare

fyrchik changed title from ~~WIP: registry: optimize object marshaling~~ to registry: optimize object marshaling

2023-03-24 12:29:10 +00:00

fyrchik requested review from storage-core-developers 2023-03-24 12:31:01 +00:00

fyrchik requested review from storage-core-committers 2023-03-24 12:31:01 +00:00

fyrchik requested review from storage-services-committers 2023-03-24 12:31:06 +00:00

fyrchik requested review from storage-services-developers 2023-03-24 12:31:07 +00:00

fyrchik requested review from abereziny 2023-03-24 12:31:18 +00:00

vdomnich-yadro approved these changes 2023-03-24 12:58:22 +00:00

vdomnich-yadro left a comment

Overall looks good, just some thoughts/observations.

internal/registry/obj_info.go Outdated

													
				@ -0,0 +42,4 @@

					o.PayloadHash = r.ReadString()

				}

				func (o *ObjectInfo) decodeFiltered(r *io.BinReader) {

vdomnich-yadro commented

2023-03-24 12:57:56 +00:00

It took a bit of code reading to figure out what this function does. I can think of two things that might help a code reader:

Either add a comment like // decodeFiltered decodes only those fields that are used in object filtering (for the sake of optimization).
Or maybe rename function to decodeFilterableFields and probably it will be sufficient without the comment.

It took a bit of code reading to figure out what this function does. I can think of two things that might help a code reader: 1. Either add a comment like `// decodeFiltered decodes only those fields that are used in object filtering (for the sake of optimization).` 2. Or maybe rename function to `decodeFilterableFields` and probably it will be sufficient without the comment.

fyrchik commented

2023-03-27 12:56:10 +00:00

Poster

Fixed

internal/registry/obj_selector.go Outdated

													
				@ -164,12 +175,13 @@ func (o *ObjSelector) selectLoop() {

				}

				func (f *ObjFilter) match(o ObjectInfo) bool {

					//fmt.Printf("filter '%s', object '%s'\n", f.Status, o.Status)

vdomnich-yadro commented

2023-03-24 12:50:29 +00:00

Looks like temp code for debuggins. Should we just remove it?

fyrchik commented

2023-03-27 12:56:13 +00:00

Poster

Fixed

internal/registry/obj_selector.go

													
				@ -165,3 +176,4 @@

				func (f *ObjFilter) match(o ObjectInfo) bool {

					//fmt.Printf("filter '%s', object '%s'\n", f.Status, o.Status)

					if f.Status != "" && f.Status != o.Status {

vdomnich-yadro commented

2023-03-24 12:53:01 +00:00

Do we still need this code here? We already pre-filter objects by status on bucket level, so maybe we can rename the function to filterAge and remove status check from here?

Not a big deal, though...

Do we still need this code here? We already pre-filter objects by status on bucket level, so maybe we can rename the function to `filterAge` and remove status check from here? Not a big deal, though...

fyrchik commented

2023-03-27 12:57:08 +00:00

Poster

We don't need it, but I would leave it here: the function could be used in any context and it is easier not to forget something, were we to change something in future.