mapbox / mapbox-sdk-cs

C# libraries for Mapbox API layer
https://mapbox.github.io/mapbox-sdk-cs/
Other
20 stars 11 forks source link

Remove reference-types from structs #63

Open david-rhodes opened 7 years ago

david-rhodes commented 7 years ago

From Unity documentation:

The way that our code is structured can impact garbage collection. Even if our code does not create heap allocations, it can add to the garbage collector’s workload.

One way that our code can unnecessarily add to the garbage collector’s workload is by requiring it to examine things that it should not have to examine. Structs are value-typed variables, but if we have a struct that contains contains a reference-typed variable then the garbage collector must examine the whole struct. If we have a large array of these structs, then this can create a lot of additional work for the garbage collector.

Tile.Parameters:

// I'm a struct . . .
public struct Parameters 
{
    public CanonicalTileId Id;

        // but I contain reference-types :(
    public string MapId;
    public IFileSource Fs;
}

Solution: pass these parameters in the constructor?

Response:

public struct Response
{
       // reference-types
    public string Error;
    public Dictionary<string, string> Headers;

    public byte[] Data;
}

Solution: Handle errors with a different pattern. Do we even use Headers?

/cc @brnkhy @isiyu @MateoV @BergWerkGIS

wilhelmberg commented 7 years ago

Solution: pass these parameters in the constructor?

Generally speaking, I prefer passing (a lot of) parameters as a single objects as it makes refactoring easier when additional parameters are needed later on as it doesn't change the signature of the method.

But yeah, if it helps here, no objection.

Do we even use Headers?

Not yet. But I put Headers there in preparation for tile fetching improvements: multithreading, error evaluation (e.g.: X-Rate-Limit-Reset, https://www.mapbox.com/api-documentation/#rate-limits) and better feedback on errors.

brnkhy commented 7 years ago

if we have a struct that contains contains a reference-typed variable then the garbage collector must examine the whole struct.

So this is same as a class right? Why are we using struct for Parameters and Response in the first place? Them being struct means whole thing is cloned every time it's passed to another function, which probably creates a bigger inefficiency to begin with.