Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Remove code that was moved to System.Native#6932

Merged
jkotas merged 4 commits into
dotnet:masterfrom
filipnavara:sysnative1
Feb 4, 2019
Merged

Remove code that was moved to System.Native#6932
jkotas merged 4 commits into
dotnet:masterfrom
filipnavara:sysnative1

Conversation

@filipnavara
Copy link
Copy Markdown
Member

Testing build. I assume the C# P/Invoke prototypes should be moved to shared CoreLib (to be in CoreFX too). Will this be ok with ProjectN?

@MichalStrehovsky
Copy link
Copy Markdown
Member

Will this be ok with ProjectN?

Yup

@filipnavara
Copy link
Copy Markdown
Member Author

filipnavara commented Feb 1, 2019

Oh, right... I totally forgot that ProjectN is Windows-only.


// Cache the frequency on the managed side to avoid the cost of P/Invoke on every access to Frequency
public static ulong Frequency { get; } = Interop.Sys.GetHighPrecisionCounterFrequency();
static HighPerformanceCounter()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Explicit static constructors are not good for performance.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What is the recommended pattern then? LazyInitializer? This class has just two fields and anyone using it is bound to use both, so I assumed it would be ok.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Non-explicit before field constructor like what was there before or like what is in the Windows version.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I see. The Windows version works for this. The Unix version cannot be used because of the out parameter in the CoreFX prototype of the native function.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Feb 1, 2019

C# P/Invoke prototypes should be moved to shared CoreLib

Yep. The best way to do that is to submit a PR to CoreFX first to move them.

@filipnavara
Copy link
Copy Markdown
Member Author

The best way to do that is to submit a PR to CoreFX first to move them.

Only two of them exist there, I can do it for them. The other two never had the C# declaration in CoreFX.

@filipnavara filipnavara changed the title WIP: Remove code that was moved to System.Native Remove code that was moved to System.Native Feb 3, 2019
Comment thread src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems Outdated
@jkotas jkotas merged commit 9764325 into dotnet:master Feb 4, 2019
@filipnavara filipnavara deleted the sysnative1 branch February 4, 2019 13:09
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/coreclr that referenced this pull request Feb 4, 2019
* Remove code that was already moved from System.Private.CoreLib.Native to System.Native.

* Move Interop files for System.Native to shared Interop directory.

* Add FeaturePortableThreadPool conditions to csproj files.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corefx that referenced this pull request Feb 4, 2019
* Remove code that was already moved from System.Private.CoreLib.Native to System.Native.

* Move Interop files for System.Native to shared Interop directory.

* Add FeaturePortableThreadPool conditions to csproj files.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request Feb 4, 2019
* Remove code that was already moved from System.Private.CoreLib.Native to System.Native.

* Move Interop files for System.Native to shared Interop directory.

* Add FeaturePortableThreadPool conditions to csproj files.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
marek-safar pushed a commit to mono/mono that referenced this pull request Feb 4, 2019
* Remove code that was already moved from System.Private.CoreLib.Native to System.Native.

* Move Interop files for System.Native to shared Interop directory.

* Add FeaturePortableThreadPool conditions to csproj files.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/coreclr that referenced this pull request Feb 4, 2019
* Remove code that was already moved from System.Private.CoreLib.Native to System.Native.

* Move Interop files for System.Native to shared Interop directory.

* Add FeaturePortableThreadPool conditions to csproj files.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
stephentoub pushed a commit to dotnet/corefx that referenced this pull request Feb 4, 2019
* Remove code that was already moved from System.Private.CoreLib.Native to System.Native.

* Move Interop files for System.Native to shared Interop directory.

* Add FeaturePortableThreadPool conditions to csproj files.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
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