Remove usages of TaskQueue #76453
Remove usages of TaskQueue #76453
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| } | ||
|
|
||
| File.AppendAllText(_logFilePath, _buffer.ToString()); | ||
| _buffer.Clear(); |
There was a problem hiding this comment.
It looks like _buffer can go away at this point; before it was collecting stuff until it got written, now it's just being used as a local here. And we could just directly do the writing to a file to avoid the giant string allocation.
There was a problem hiding this comment.
Looks like it's trying to do a single write, instead of N smaller writes. I have no issue keeping this.
There was a problem hiding this comment.
I kind of like Jason's suggestion too, it can still be a single call into the File API, something like:
var now = DateTime.Now;
File.AppendAllLines(_logFilePath, list.Select((functionId, message) => $"{now} ({functionId}) : {message}"));
There was a problem hiding this comment.
Can we separate out core parts of PRs, vs nice to haves? I'm fine with potentially making this change in a future PR, or having one of you guys do it. The core goal here is simplifying the threading primitives. I don't want to be iterating a lot on making extra changes that take away from that work.
Please separate out nits and suggestions, from requirements for a PR to go in. Thanks @jasonmalinowski @ToddGrun
There was a problem hiding this comment.
@CyrusNajmabadi Even if we don't want to optimize, at least move _buffer to a local. Otherwise we're leaving a field around that doesn't need to be a field, and looks like there's more complexity than there is.
There was a problem hiding this comment.
I can make this change in a followup PR.
No description provided.