Move GetCurrentProcessId interop for Browser to managed #39367
Conversation
It returns constant all the time
jkotas
added
area-System.Threading
area-System.Runtime
arch-wasm
|
How much does this save? Is it really worth it to be doing micro-optimizations like this? |
|
@jkotas it does not save much but I'd rather have these constants under our control and not depend on Emscripten |
| return new OperatingSystem(PlatformID.Other, new Version(1, 0, 0, 0)); | ||
| } | ||
|
|
||
| private static int GetCurrentProcessId() => (int)Interop.GetCurrentProcessId(); |
There was a problem hiding this comment.
This is the one and only place Interop.GetCurrentProcessId is used. Hardcode 42 here instead of going through extra layer of indirection?
It would match the hardcoding that we are doing for the other properties above. The other properties are not going through extra layer of indirection either.
There was a problem hiding this comment.
Good suggestion (not sure why I missed that)
There was a problem hiding this comment.
@jkotas looking at the build failure I know now why it was done this way. There are direct calls to Interop.GetCurrentProcessId within Microsoft.Diagnostics.Tracing.EventSource project. Are you ok with revert of the last commit?
There was a problem hiding this comment.
I have pushed commit that should fix the EventSource build break
* Move GetCurrentProcessId for Browser to managed It returns constant all the time * Remove redundant indirection * Fix EventSource build break Co-authored-by: Jan Kotas <[email protected]>
ghost
locked as resolved and limited conversation to collaborators
4 participants
It returns constant all the time