Skip to content

Add simple caching into enrichers #16

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

Merged
merged 1 commit into from
Sep 2, 2019
Merged

Conversation

sungam3r
Copy link
Contributor

@sungam3r sungam3r commented Sep 2, 2019

Also fixes readme formatting.

Before:
image
After:
image

fix readme formatting
@sungam3r
Copy link
Contributor Author

sungam3r commented Sep 2, 2019

@nblumhardt This caching will manifest itself the more, the less the switching of threads in the application. In a single-threaded application there will be 100% hits. It does not add a lot of load.

@nblumhardt
Copy link
Member

Very interesting! The lack of synchronization on _lastValue is a sneaky optimization... 😄 (as you'd have noted, if the value ends up cached in a register and isn't updated from memory despite concurrent writes by other threads, the hit rate seems like it might actually be higher than expected.)

My gut feel is that this will be a positive, it's tricky to gauge the impact without benchmarks, but then, it's also nearly impossible to benchmark meaningfully without a huge amount of effort. Let's do it! 👍

@nblumhardt nblumhardt merged commit a049f43 into serilog:dev Sep 2, 2019
@sungam3r
Copy link
Contributor Author

sungam3r commented Sep 3, 2019

it's tricky to gauge the impact without benchmarks, but then, it's also nearly impossible to benchmark meaningfully without a huge amount of effort.

Well, it is possible. But in this respect, and without a benchmark, it is clear that there will be fewer allocations, the whole point is in your own desire to see a performance increase. 5, 10, or 99% reduction in allocations - the specific number is not so important, it can vary from scenario. The approach itself is important - do not allocate the same object over and over again.

@sungam3r sungam3r deleted the caching branch September 3, 2019 06:11
@nblumhardt nblumhardt mentioned this pull request Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants