Skip to content

Remove capture allocation from WriteArray #20775

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.

Sign up for GitHub

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
Jul 11, 2017
Merged

Remove capture allocation from WriteArray #20775

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Compilers/Core/Portable/Serialization/ObjectWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,8 @@ private void WriteArray(Array array)
// don't blow the stack. 'LongRunning' ensures that we get a dedicated thread
// to do this work. That way we don't end up blocking the threadpool.
var task = Task.Factory.StartNew(
() => WriteArrayValues(array),
a => WriteArrayValues((Array)a),
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Jul 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait... doesn't this still need to capture 'this'?

Copy link
Member Author

@davkean davkean Jul 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyrusNajmabadi and I spoke offline. There's still an allocation of a delegate - it doesn't appear to be showing at all on the radar. I looked at the code gen and figured out why:

  • The first thing that the compiler does in the method is instantiate the capture class and "capture" array.
  • It then replaces all reads/writes of the parameter to the field in the capture class

Basically, in summary it looks like this method is always paying for the capture class regardless of the branch. That's why the delegate creation itself isn't showing - the branch is never hit.

array,
_cancellationToken,
TaskCreationOptions.LongRunning,
TaskScheduler.Default);
Expand Down