TileData + Worker refactor

[jfirebaugh]

Cleanly separate two different kinds of data:

Map Tile Data	Worker Tile Data
Affiliated with the Map thread	Affiliated with one of the Worker threads
May include GL objects	"Plain old data" only; no GL objects (#926)
Owned by a Tile object	Owned by a TileWorker object
	May be moved to map thread at key moments (e.g. after a tile parse completes)

Data will move between map thread and worker threads via the Thread::invoke message passing system. No tile data will be shared between the two. (For now we'll continue to share references to Style and atlases, but this is another step toward removing that sharing as well.)

Targeted bugs:

• Don't create GL objects in the tile worker thread #926
• Remove concurrent access to objects from worker threads #667 (partially)

Plan of attack:

• Collapse TileData inheritance hierarchy into a fully-abstract base class and RasterTileData, VectorTileData, LiveTileData child classes. Note that LiveTileData will not inherit from VectorTileData. (This will introduce code duplication that will later be factored away.)
• Introduce TileWorker class, move worker code from TileData to it. This may need to be a parallel inheritance hierarchy with RasterTileWorker, VectorTileWorker, and LiveTileWorker. At first objects of this class will be owned by TileData and descendants, but eventually they will live persistently on the worker thread.
• Move the making of tile requests to TileWorker.
• Add smarter dispatching to Worker. Assign a worker thread to a tile once, and then dispatch future work requests to the same worker thread, like in gl-js.
• Migrate TileWorker to worker threads.

There are many known unknowns here -- best way to learn them is to start working on this and see what we run into.

[jfirebaugh]

First obstacle to flattening the inheritance hierarchy is that TileParser depends on VectorTileData, but is used by both VectorTileData and LiveTileData. TileParser uses the following:

• tile.getState()
• tile.getBucket()
• tile.setBucket()
• tile.id
• tile.fillVertexBuffer, tile.triangleElementsBuffer, etc
• tile.getCollision()
• reinterpret_cast<uintptr_t>(&tile) (for GlyphAtlas indexing)

These are all the types of things that I expect to migrate into TileWorker, so I'm going to try introducing that now.

[tmpsantos]

These are all the types of things that I expect to migrate into TileWorker, so I'm going to try introducing that now.

Hmm, so the TileWorker is not exactly a Worker. I got a bit confused at the beginning but after reading the code is quite clear to me, but maybe we should call it something like TileWorkerJob.

[tmpsantos]

@jfirebaugh 👍

[jfirebaugh]

Right, TileWorker is an interim class. Once it's fully separable from the TileData hierarchy it should merge into Worker::Impl. Then we should probably rename Worker to TileWorker.

[kkaefer]

These comments are very helpful. Any reason to remove them?

[jfirebaugh]

My eventual plan was to have a flat hierarchy, with LiveTileData not inheriting from VectorTileData, so that (among other things) this would become moot. If we end up not doing that I'll add the comments back.

[jfirebaugh]

I'm currently working on moving the after callback / canceling semantics offered by WorkTask into the core RunLoop implementation -- into Invoker specifically.

This involves adding an additional template argument to Invoker which is instantiated with the callback type, either std::function<void (R)> for some type R if the invoked method returns a result, or std::function<void ()> if it does not. I then want to overload the invoke member function with implementations for these two cases:

        template <std::size_t... I>
        void invoke(std::index_sequence<I...>) {
            func(std::forward<Args>(std::get<I>(params))...);
            callback();
        }

        template <std::size_t... I>
        void invoke(std::index_sequence<I...>) {
            callback(func(std::forward<Args>(std::get<I>(params))...));
        }

I need to use some kind of enable_if or SFINAE to resolve the overloading but I'm stuck on that. Basically I want the first definition to be used for std::function<void ()> and the second one for std::function<void (R)>.

[kkaefer]

Assign a worker thread to a tile once, and then dispatch future work requests to the same worker thread, like in gl-js.

Why is this necessary?

[jfirebaugh]

As you pointed out in #1471 (comment), it's probably not, and I agree.

[jfirebaugh]

This branch is currently hanging on headless rendering tests. It has something to do with reparsing partial tiles, but the data flow there is really hard to follow. @tmpsantos do you think you could take a look at it?

[tmpsantos]

This branch is currently hanging on headless rendering tests. It has something to do with reparsing partial tiles, but the data flow there is really hard to follow. @tmpsantos do you think you could take a look at it?

On it.

[kkaefer]

@tmpsantos I would love the partial parsing to be refactored. I'm thinking we should create a list of all layers that need parsing when we start the parse step. In the parse thread, we iterate through that list, parse, then remove from that list when we've parsed a layer. This means that at some point, this list will be empty and we know that everything's been parsed. This is likely less error prone than using the total bucket count as a measure for whether we need to parse again. This also means that we can start removing the state property.

[kkaefer]

The long term goal is to split up parsing per layer so that we could ideally parse one vector tile with multiple threads. This requires making the task of parsing a layer from a tile independent of order and ideally has no concurrent access. This means we'd have to give every layer their own buffers and not do label placement until we actually have the label layers parsed.