Fix #779: NRE on Dispose in ExecutionTimer #782
Conversation
|
|
||
| namespace Microsoft.PowerShell.EditorServices.Utility | ||
| { | ||
| public class ObjectPool<T> |
There was a problem hiding this comment.
The build is currently failing because XML comments are missing for these types.
ObjectPool should be internal I imagine, but it would also be worthwhile to comment them to explain their basic semantics if you've got time.
There was a problem hiding this comment.
Also, the fact that the XML comments fails a Release build but not a Debug build is something we need to fix :)
There was a problem hiding this comment.
I mean I understand why you could prefer not having these warnings as errors when you're developing. Running the release build locally before pushing is a good habit but it requires net451 though and it can't run on my linux machine :)
There was a problem hiding this comment.
Yeah, I hit similar problems as well when I was building on Linux.
I just never think to run a release build and then CI fails for something I wish I'd at least been warned about locally...
Anyway, the work to move off net451 has already been done downstream in the v2 branch, so soon enough it will be easy for Linux and macOS users to have exactly the same build experience as Windows.
There was a problem hiding this comment.
I'm pretty happy with this.
Will wait for @tylerl0706 and ideally @SeeminglyScience to sign off too.
@daxian-dbw raised the possibility that just calling new Stopwatch() might not be that expensive and allows us to avoid complexity. But given that the cleverer implementation is done, I'm happy either way.
|
Thanks so much for your help @dee-see! |
|
My pleasure! The objective is to finally do what I said I would do almost a year ago: resolve #17 :) |
Fix #779: NRE on Dispose in ExecutionTimer
As discussed,
ThreadStaticimplementation replaced by an object pool. My object pool is very basic as I didn't feel like the current usage warranted anything more complex. Let me know if you think I should do anything different!