Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Faster CopyFrom(+Ascii)#585

Closed
benaadams wants to merge 1 commit into
aspnet:devfrom
benaadams:copyfrom-perf-finalized
Closed

Faster CopyFrom(+Ascii)#585
benaadams wants to merge 1 commit into
aspnet:devfrom
benaadams:copyfrom-perf-finalized

Conversation

@benaadams
Copy link
Copy Markdown
Contributor

Fixed finalizer block return issue in #527 by making the finalizer using MemoryPoolBlock2.Create rather than .ctor directly

Resolves #579

Fixed finalizer block return; by reusing Create rather than .ctor
@halter73
Copy link
Copy Markdown
Member

I was kind of kicking myself for merging the earlier version of this PR last week, so I'm a wary of merging it again. When I tested this before, I remember seeing a little over a 1% RPS improvement in the plaintext benchmark. Your own numbers show about a 2.5% improvement. Either way, I don't think this improvement is dramatic enough for taking a perf related change at this point. I'm worried that there might be yet another undiscovered bug.

I feel bad about this because I've pretty much already reviewed and accepted this PR. It's definitely something I will likely merge post 1.0.

@halter73 halter73 closed this Jan 18, 2016
@benaadams
Copy link
Copy Markdown
Contributor Author

Np, completely understand :)

@benaadams
Copy link
Copy Markdown
Contributor Author

On the plus side, merging help identify and issue and showed the finalizer was doing critical work - so it ended up being a good thing 😜

@benaadams benaadams deleted the copyfrom-perf-finalized branch May 10, 2016 11:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants