Skip to content

Conversation

@jkotas
Copy link
Member

@jkotas jkotas commented Dec 6, 2023

No description provided.

@ghost ghost added the area-VM-coreclr label Dec 6, 2023
@ghost ghost assigned jkotas Dec 6, 2023
@hughbe
Copy link
Contributor

hughbe commented Dec 6, 2023

Jan - is there an issue/PR somewhere that explains the rationale behind these changes (perf, correctness, other?)? just curious!

@DragosDanielBoia
Copy link
Contributor

@dotnet-policy-service rerun

@am11
Copy link
Member

am11 commented Dec 6, 2023

@hughbe, perhaps there is no tracking issue, but I think this is a general goodness to have fewer FCalls as possible https://github.com/dotnet/runtime/blob/b345e2dbd6936eb281fadb7d70473358be578d24/docs/design/coreclr/botr/corelib.md#choosing-between-fcall-qcall-pinvoke-and-writing-in-managed-code. The ultimate goal is to gradually get rid of HELPER_METHOD_FRAME (dotnet/corert#3784 (comment)), and thereby reduce/remove the dependency on libunwind from Unix.

@jkotas, are we strictly converting FCalls to QCalls in this series, or to managed as well (when there is an opportunity)?

@jkotas
Copy link
Member Author

jkotas commented Dec 6, 2023

Jan - is there an issue/PR somewhere that explains the rationale behind these changes (perf, correctness, other?)?

Good point. I have opened #95695. It is something that myself, @AaronRobinsonMSFT, @davidwrighton discussed over lunch as a general goodness a few weeks ago. I have decided to work on it as my side-project.

@jkotas
Copy link
Member Author

jkotas commented Dec 6, 2023

@jkotas, are we strictly converting FCalls to QCalls in this series, or to managed as well (when there is an opportunity)?

Yes, if there is an easy opportunity. There are a few cases like that in this PR:

  • ArgIterator.GetRemainingCount - move to managed
  • StubHelpers - move a call of managed pointer to managed
  • Moved argument checking to managed in some places

@davidwrighton
Copy link
Member

This looks good although I might instead of converting the trailing byte code into a QCall instead produce a QCall/FCall that can produce the SyncBlock and then do the rest in managed. However, we could tackle that later.

@jkotas
Copy link
Member Author

jkotas commented Dec 7, 2023

I might instead of converting the trailing byte code into a QCall instead produce a QCall/FCall that can produce the SyncBlock and then do the rest in managed. However, we could tackle that later.

Yes, it is a trade-off in how many data type definition to mirror between managed and unmanaged sides. I have been leaning towards keeping this mirroring more minimal.

@jkotas jkotas merged commit 6dc1087 into dotnet:main Dec 8, 2023
@jkotas jkotas deleted the hmf4 branch December 8, 2023 02:43
@github-actions github-actions bot locked and limited conversation to collaborators Jan 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants