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

CreateEventIdProperty: consider caching EventId properties #118

Closed
cd21h opened this issue May 1, 2018 · 3 comments
Closed

CreateEventIdProperty: consider caching EventId properties #118

cd21h opened this issue May 1, 2018 · 3 comments

Comments

@cd21h
Copy link

cd21h commented May 1, 2018

EventId is immutable class and typically is instantiated as singleton.
Caching log properties based on EventId CreateEventIdProperty may reduce memory usage.

However cache size must be limited in case EventId is generated dynamically

@nblumhardt
Copy link
Member

Thanks for the note!

It's an interesting possibility, but there are often non-obvious performance implications of caching, so we'd first need data showing that there's a substantial allocation overhead to be saved, here, and that the caching can improve on it without other impacts.

Are you using EventId heavily in your own code, or is it mostly framework code that you think will benefit? Cheers!

@cd21h
Copy link
Author

cd21h commented May 24, 2018

Hi,
sorry for delay with response.

Is is extensively used by ASP.NET Core (just an Id) and our system (both Id and Name).

@cd21h
Copy link
Author

cd21h commented May 24, 2018

I run a quick test for one of my app. After running for several minutes under load I found 12K LogEventProperty objects were created / collected. Examining log file I found less than 50 unique event Ids.

image

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