Store gfx::DrawScope objects with associated render objects.

[kkaefer]

We used some shared SegmentVectors, e.g. for drawing raster or background tiles. In longer running maps, this lead to resource accumulation. By storing the SegmentVectors and the contained gfx::DrawScope objects, we ensure that resources get released when the associated render objects vanish.

As a longer term solution, we should remove gfx::DrawScope objects from SegmentVector objects and store them separately.

Fixes: #15179

[pozdnyakov]

Looks great overall!

[springmeyer]

In longer running maps, this lead to resource accumulation.

What is the best way to detect a situation like this in general? Is it possible to have a test that runs on CI which could catch a situation like this?

As a longer term solution, we should remove gfx::DrawScope objects from SegmentVector objects and store them separately.

What is the reason not to do that now? Too much effort or is it blocked by something specific currently?

[kkaefer]

What is the reason not to do that now? Too much effort or is it blocked by something specific currently?

I embarked on this refactor, but it got too convoluted in the symbol rendering code due to too many code paths, so I decided to split this into multiple PRs and address the issue separately.

[kkaefer]

What is the best way to detect a situation like this in general? Is it possible to have a test that runs on CI which could catch a situation like this?

I've been using Instruments.app and Mark Generation, then manually inspecting the remaining allocations while running the node-benchmark target.

I'm not sure there's a good way to automate this without lots of false positives in CI environment; I've been seeing various V8-internal allocations that I attributed to its memory management, as well as memory that was still allocated for good reasons (e.g. system caches of decompressed images).

[springmeyer]

I'm not sure there's a good way to automate this without lots of false positives in CI environment; I've been seeing various V8-internal allocations that I attributed to its memory management, as well as memory that was still allocated for good reasons (e.g. system caches of decompressed images).

Got it. Do you think we might avoid false positives if we had a pure C++ benchmark instead of using a node one (obviously would still have issue of system caches)?

[springmeyer]

@kkaefer is this close to ready to merge? I'll leave it to @pozdnyakov or another @mapbox/gl-native dev to approve.

[springmeyer]

bump @kkaefer

[kkaefer]

I'm trying to figure out why https://circleci.com/gh/mapbox/mapbox-gl-native/317751 fails; it has repeatedly failed on CI at combinations/hillshade-translucent--line-translucent, which is just blank. That test doesn't fail locally for me.

[springmeyer]

Thanks for landing this @alexshalamov!