search

serilog / serilog-extensions-logging

Serilog provider for Microsoft.Extensions.Logging
Apache License 2.0
309 stars 99 forks source link

CreateEventIdProperty: consider caching EventId properties #118

Closed cd21h closed 4 years ago

cd21h commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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