Don't use io.ReadAll while recycling indices

[mtojek]

This PR replaces io.ReadAll with json.Decode as the routine doesn't need the index content. I'm not convinced about this structure as it uses json.Unmarshaller, but I don't have an idea how to improve it.

Right now:

If you can observe some other places to refactor, feel free to comment.

Testing (if you want to play):

go run . -feature-storage-indexer -storage-indexer-bucket-internal gs://mtojek-temp/package-storage-v2 -log-level debug -httpprof ":6060"

mtojek-temp is in the elastic-observability namespace.

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-09-20T11:09:59.017+0000

• Duration: 5 min 34 sec

Test stats 🧪

Test	Results
Failed	0
Passed	213
Skipped	0
Total	213

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

[jsoriano]

I'm not convinced about this structure as it uses json.Unmarshaller, but I don't have an idea how to improve it.

These calls to json.Unmarshaller will be done for each package JSON one after the other, this should be fine memory-wise. Have you found performance problems there?

If you can observe some other places to refactor, feel free to comment.

Other place that may be copying too many objects is transformSearchIndexAllToPackages, that seems to be copying all package manifests.
Maybe this transformation could be done directly by loadSearchIndexAll. Or searchIndexAll struct could contain pointers to manifests that can be reused in the transform step.

[jsoriano]

👍 only by not keeping the whole index response in memory there should be improvements in memory usage.

[mtojek]

These calls to json.Unmarshaller will be done for each package JSON one after the other, this should be fine memory-wise. Have you found performance problems there?

It depends on how json manages the decode cache inside, but I'm not master of that area, so can't answer if we can spot fields to improve there.

Other place that may be copying too many objects is transformSearchIndexAllToPackages, that seems to be copying all package manifests.

Agree, but I don't see this part "hot" in the calltree graph. Only 500 KB is used there.

I don't know if there is any json marshaller that deallocates used memory. I tried to play with jsoniter, but it gave similar results (as the logic doesn't jump over fields).

Still 👍 to merge it, @jsoriano? :)

[jsoriano]

Still +1 to merge it, @jsoriano? :)

Change makes sense to me, so ok to merge :)

Though not sure if it will solve all the problems. But we can iterate. Do you have a profile with the new code?

[mtojek]

Do you have a profile with the new code?

Do you mean http://localhost:6060/debug/pprof/heap? Yes, I dumped it, but it's easy to generate it just in case.

[mtojek]

Just in case, before introducing this change: