Skip to content
This repository has been archived by the owner on Nov 25, 2019. It is now read-only.

Consider StringBuilder to build request URLs #64

Open
david-rhodes opened this issue Apr 4, 2017 · 4 comments
Open

Consider StringBuilder to build request URLs #64

david-rhodes opened this issue Apr 4, 2017 · 4 comments

Comments

@david-rhodes
Copy link
Contributor

We have loads of string manipulation happening throughout the SDK to format request URLs. Because strings are immutable, this is creating lots of garbage for each request. I'm guessing each request uses 20+ temporary strings. Request 128 tiles and now you have problems.

Solution: use StringBuilder (and dependency injection?)

@wilhelmberg
Copy link
Contributor

Or UriBuilder which seems to be appropriate for url manipulation.
Maybe even in combination with HttpUtility.UrlEncode.

@brnkhy
Copy link
Collaborator

brnkhy commented Apr 5, 2017

intantiating a new string builder won't be cheap either though. It'll also have to be collected just like string but probably even more costly. Maybe we can do some tricky stuff like having a static string builder in the IFileSource but such tricks are generally more prone/easier to introduce new bugs.

Skeet's article about string concetination; http://jonskeet.uk/csharp/stringbuilder.html
I say we can try it and compare performance but I also think the performance different will be quite quite low and considering how much time it takes to monitor&compare performance results, we might not prioritize this.

@david-rhodes
Copy link
Contributor Author

@brnkhy I read that just 3 separate concatenation statements justify the cost of the string builder object. That said, I was wondering how well a static string builder would work . . .

Go through and count how many concatenations happen to format just one URL. This is non-trivial if you want a performant slippy map.

@brnkhy
Copy link
Collaborator

brnkhy commented Apr 5, 2017

@david-rhodes admittedly I'm not sure what to do about these stuff these days considering that they already announced .net 4.6 for unity. i.e. unity will be getting async programming with that, which might be the best replacement for tile.initialize we're talking in other ticket. Unity garbage collector will most likely change as well so all these micro optimizations, even though quite important for core, might a waste of time.
Unity is targeting June for 2017 release by the way, not that far away either.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants