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

Optimizing Expression.Compile computation #141

Merged
merged 1 commit into from Aug 24, 2017

Conversation

Projects
None yet
3 participants
@darkl
Copy link
Contributor

commented Aug 4, 2017

There is no need to call Expression.Compile more than once per expression. This pull request moves some of these computations from instance context/message context to static context.

Elad

@llehn llehn referenced this pull request Aug 4, 2017

Closed

LibLog performance #129

@dadhi

This comment has been minimized.

Copy link

commented Aug 5, 2017

Hi, Expression.Compile just caught my attention :)

May be it may be improved further by using FastExpressionCompiler. I am an owner.

Recently was used in Marten 2.0.

@darkl

This comment has been minimized.

Copy link
Contributor Author

commented Aug 5, 2017

Cool! I think this isn't really suitable for LibLog as it is supposed to be a self-contained (i.e. does not require any dependencies) single .cs file. Why don't you propose these optimizations to corefx?

Elad

@dadhi

This comment has been minimized.

Copy link

commented Aug 5, 2017

Actually, FEC is the single .cs file. So you may use/abuse/embed it as is. I do this myself in DryIoc, and know others do.

Why not to propose to CoreFx? Cause it is not cover-everything lib. It optimizes a range of things it knows about and fallbacks to the core for the rest.

@darkl

This comment has been minimized.

Copy link
Contributor Author

commented Aug 5, 2017

@damianh

This comment has been minimized.

Copy link
Owner

commented Aug 24, 2017

Nice, thanks for the PR!

@damianh damianh merged commit 08ea017 into damianh:master Aug 24, 2017

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

bbrandt added a commit to bbrandt/LibLog that referenced this pull request Sep 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.