Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Suggestion] Use the String Interning for Candlestick.Symbol property. #84

Closed
sguryev opened this issue Apr 6, 2018 · 6 comments
Closed
Assignees

Comments

@sguryev
Copy link

sguryev commented Apr 6, 2018

I'm making a lot of REST API calls as well as WebSocket connections. dotMemory says that I can optimize memory by using String Interning for frequently used string values (just imagine - 1000 candlesticks with the same symbol ETHBTC x intervals)
image

Article to read: https://blog.jetbrains.com/dotnet/2015/02/12/string-interning-effective-memory-management-with-dotmemory/

@sguryev sguryev changed the title [Suggestion] Use the String Interning for Candlestick.Symbol property. [Suggestion] Use the String Interning for Candlestick.Symbol property. Apr 6, 2018
@sguryev
Copy link
Author

sguryev commented Apr 6, 2018

Actually it would be awesome to use the string.Intern() as much as possible

@sonvister sonvister self-assigned this Apr 6, 2018
sonvister added a commit that referenced this issue Apr 6, 2018
For #84. Update static assets and symbols.
@sonvister
Copy link
Owner

@sguryev, good suggestion. I've added string interning to the assets and symbols. Feel free to close this issue when you can verify the memory usage is optimized with latest code or the next release. Thanks.

@sguryev
Copy link
Author

sguryev commented Apr 7, 2018

Hmmm... I have left a couple of comments. They says that literals are interned by default. Since you have all the possible symbols as literals in the Cache they should be interned in all other places: (Binance API response deserialization, etc). Not sure if it work for the Dictionary keys though.

@sguryev
Copy link
Author

sguryev commented Apr 7, 2018

You have the code generation there. Is it design time generator?

sonvister added a commit that referenced this issue Apr 7, 2018
@sonvister
Copy link
Owner

@sguryev, your're right about string literals; I've corrected. Thanks.

Yes, the known symbols will be interned by the static Cache declaration. All other changes (where strings are modified/generated) remain since we also want to handle new assets and symbols (created when the cache is updated), which is also why I didn't use IsInterned.

The BinanceCodeGenerator program is used to get the latest assets and symbols and generate the Asset.cs and Symbol.cs files. Had that been more tedious to implement I may have looked up automatic interning of string literals beforehand 😉

@sguryev
Copy link
Author

sguryev commented Apr 22, 2018

I think we can close the issue now. Thank you!

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

No branches or pull requests

2 participants