Improve Data Fetching in Unity

[david-rhodes]

TL;DR

Our current Unity implementation for making web requests is quite inefficient. Let's find ways to improve it.

Possible Inefficiencies

• Web requests are using coroutines. Each coroutine allocates memory on the heap (every tick). Requesting hundreds of tiles will lead to "massive" GC spikes.
• Requests are not threaded.
• Each low level object that wants to make a request needs a FileSource. This is often duplicated between objects. Unknown what the correct solution is, but probably depends on our threading options.
• Currently, response errors are handled as strings. String compares are expensive and prone to error.

Other Considerations

• Coroutines do not operate (sans hacks) at edit time. This makes it difficult to perform requests outside of "play" mode.
• I expect developers to hit their rate limits quite easily. It would be nice if we offered a convenience layer to automatically handle the distribution of requests or re-queuing of requests on rate limit exceeded errors. Of course a lot of developers will implement their own version of this to suit their needs, but ours may serve as a solid starting point.
• Web requests should have timeouts (I have, on occasion, observed requests never providing a response).

Possible Solutions

• Use something like More Effective Coroutines (note: unsure if this will also solve the editor coroutine issues, but it could since you don't need to inherit from MonoBehaviour).
• Use C# Threads.
• Use System.Web for web requests (possibly less overhead).
• @BergWerkGIS mentioned using BruTile as a possible library for managing batch requests.

Next Steps

It would be great to have a C# expert (@BergWerkGIS, I'm looking at you) experiment in this area.

/cc @mapbox/games

[brnkhy]

Each low level object that wants to make a request needs a FileSource. This is often duplicated between objects.

If I get that right, this should be up to developer and they can easily pass same filesource to all services. At least that's what I'm doing in my demos (TerrainDemoUnity branch).

I also tried running everything in editor mode but as you said it doesn't work without some ugly hacks. But I think that's Unity3D's problem and we can go with them.

convenience layer

I totally agree on this one. People will feel lost without proper feedbacks, info etc. But that also means lots of editor script work I guess (or we can just debug.log for a while)

@david-rhodes we can implement both options (coroutines and threads) as filesources right? I think we should have both options anyway.

[david-rhodes]

If I get that right, this should be up to developer and they can easily pass same filesource to all services. At least that's what I'm doing in my demos (TerrainDemoUnity branch).

Sort of. I'm personally using a "singleton" FileSource, but if we add threading, I think it should be hidden from developers. They make a request through a singleton and we make a new thread if needed. Something like that.

But I think that's Unity3D's problem and we can go with them.

Not sure what "go with them means." This is our problem, because we're going to want the ability to make web requests at edit time (custom editors/property drawers, etc.).

I think we should have both options anyway.

True. This would just be another implementation of IFileSource.

[tmpsantos]

Web requests are using coroutines. Each coroutine allocates memory on the heap (every tick). Requesting hundreds of tiles will lead to "massive" GC spikes.

The file source could implement a queue in this case to throttle the max number of requests. We do that on Mapbox GL Native. Often when you are zooming or panning the map, the requests in the queue are cancelled and they never really happen.

We started with co-routines because it was the only way to support the WebPlayer. The interesting thing about the FileSource abstraction is that you could replace it with a ThreadedFileSource if you will never target the WebPlayer.

[wilhelmberg]

"bookmarking" for myself: https://github.com/BruTile/BruTile/issues/10 https://github.com/FObermaier/DotSpatial.Plugins/blob/master/DotSpatial.Plugins.BruTileLayer/TileFetcher.cs

[parahunter]

In regards to the coroutines not running outside play mode here's some ways to solve it

1. Move that code into separate threads as you have already suggested
2. Where you create the coroutine, detect if editor is playing, if not pass it onto your own coroutine runner that uses EditorApplication.Update to run the coroutine. You can use [InitializeOnLoad] to subscribe to that callback when the Unity editor is opened. Not sure if WWW will run outside play mode though but usually Unity is good at making sure everything that runs in play mode can also be done in edit mode
3. block the main thread until the data is returned if the call is done in edit mode (ugly)

[wilhelmberg]

Tried an easy route for a quick win via the backported Task Parallel Library replacing this foreach with Parallel.ForEach.

Not working 😢:

InternalCreateBuffer can only be called from the main thread.
Constructors and field initializers will be executed from the loading thread when loading a scene.
Don't use this function in the constructor or field initializers, instead move initialization code to the Awake or Start function.
UnityEngine.Networking.UnityWebRequest:Get(String)
Mapbox.Unity.HTTPRequest:.ctor(MonoBehaviour, String, Action`1) (at Assets/Mapbox/Core/Unity/HTTPRequest.cs:21)
Mapbox.Unity.FileSource:Request(String, Action`1) (at Assets/Mapbox/Core/Unity/FileSource.cs:78)
Mapbox.Map.Tile:Initialize(Parameters, Action) (at Assets/Mapbox/Core/Map/Tile.cs:87)
Mapbox.Map.<Update>c__AnonStorey0:<>m__1(CanonicalTileId) (at Assets/Mapbox/Core/Map/Map.cs:246)
System.Threading.Tasks.ThreadPoolTaskScheduler:TaskExecuteWaitCallback(Object)

[wilhelmberg]

Interesting, above error occurs with Slippy example but not with VoxelWorld. Looking for differences ....

[wilhelmberg]

Argh, VoxelWorld requests only one tile (=> no additional threads), that's why it works.

[david-rhodes]

@BergWerkGIS This may be helpful: http://blog.theknightsofunity.com/using-threads-unity/

I don't know how this Task Parallel Library works, but we may have to create our own thread and control what tasks get sent to it. Unity has a lot of oddities when dealing with threads. @parahunter might be able to help here, too.

[wilhelmberg]

@david-rhodes thanks, also tried this (same error):

            foreach (var id in cover)
            {
                Tile.Parameters param;
                param.Id = id;
                param.MapId = this.mapId;
                param.Fs = this.fs;

                Thread worker = new Thread(getTile);
                worker.IsBackground = true;
                worker.Start(param);
            }

Reading your link now ...

[david-rhodes]

@BergWerkGIS, @parahunter suggested that the issue might be using native Unity objects that are not thread safe, such as WWW. Perhaps we start by adding another implementation of web requests before attempting to thread?

[wilhelmberg]

@david-rhodes I now know what the problem is, it's the same like with updating those good ol' WinForms from another thread.

Slippy example works with UnityToolbag.Dispatcher from your comment above and this quick'n'dirty hack to Core/Map.cs:

            foreach (var id in cover)
            {
                Tile.Parameters param;
                param.Id = id;
                param.MapId = this.mapId;
                param.Fs = this.fs;

                Thread worker = new Thread(getTile);
                worker.IsBackground = true;
                worker.Start(param);
            }

        private void getTile(object objParam)
        {
            Tile.Parameters param = (Tile.Parameters)objParam;
            UnityEngine.Debug.LogFormat("{0}: creating T()", param.Id);

            if ("0/0/0" == param.Id.ToString())
            {
                UnityEngine.Debug.LogFormat("{0}: aborting", param.Id);
                return;
            }
            var tile = new T();

            UnityToolbag.Dispatcher.InvokeAsync(() =>
            {
                UnityEngine.Debug.LogFormat("{0}: tile.Initialize", param.Id);
                tile.Initialize(param, () => { this.NotifyNext(tile); });
                UnityEngine.Debug.LogFormat("{0}: AFTER tile.Initialize", param.Id);

                this.tiles.Add(tile);
                UnityEngine.Debug.LogFormat("{0}: NotifyNext", param.Id);
                this.NotifyNext(tile);
            });
        }

[david-rhodes]

@BergWerkGIS There's a note in that link:

Dispatcher is a class that is included in UnityToolbag library. It checks if the code passed to dispatcher is always executed in the main thread, so you can safely execute Unity API code within.

Does this mean our requests are not necessarily threaded, now? Did you profile or check if there's some way to see if the tasks are in fact being threaded properly?

[david-rhodes]

@BergWerkGIS, ss part of this ticket, please keep an eye on whether it makes sense to create multiple FileSource objects. Currently, this is a singleton, but that may not make sense with the final architecture.

[wilhelmberg]

Does this mean our requests are not necessarily threaded, now?

With the snipped above (new Thread(getTile)) requests are threaded. Before they weren't.

Did you profile or check if there's some way to see if the tasks are in fact being threaded properly?

Not really, but the Debug.Logs for different tiles are not in order anymore - I guess that means the request are threaded properly 😄

ss part of this ticket, please keep an eye on whether it makes sense to create multiple FileSource objects. Currently, this is a singleton, but that may not make sense with the final architecture.

That's what I'm thinking too. From my point of view most of the logic should live in a central place: probably Core/Map.cs as I think requesting just a single tile will be the exception and most users will use a map. So single tiles need less information about themselves.

Map.cs should keep track of:

• extent of current view
• fetching tiles (with background threads, not too many though 😏)
• caching tiles: just memory or file system too???
• retrying failed tiles
• throttling tile requests
• aborting tile requests
• reporting progress of tile loading
• ...

[david-rhodes]

As per our scrum talk today regarding platform compatibility:

• Please link the reference ticket here about WebGL compatibility
• I've handled these issues in the past using [Conditional] attributes to inject the appropriate "strategy" object based on platform
  • This would mean making a concrete implementation for each version that needs its own logic
  • The Map.cs object or whoever would delegate to this object.

[wilhelmberg]

Please link the reference ticket here about WebGL compatibility

@tmpsantos 's comment near the top of this thread:

We started with co-routines because it was the only way to support the WebPlayer. The interesting thing about the FileSource abstraction is that you could replace it with a ThreadedFileSource if you will never target the WebPlayer.

[wilhelmberg]

Did you profile or check if there's some way to see if the tasks are in fact being threaded properly?

Order threads were created:

Order tiles arrived:

[wilhelmberg]

To be continued in #46