Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Determinism fixes #306

Merged
merged 10 commits into from
Dec 3, 2019
Merged

Determinism fixes #306

merged 10 commits into from
Dec 3, 2019

Conversation

SrivastavaAnubhav
Copy link
Contributor

  • NativeFormatWriter's HashTable save now uses OrderBy, so the output deterministic if the values are entered deterministically
  • Make GetILBytes in EcmaMethodIL threadsafe so we don't accidentally get two different addresses for the same code
  • Fix ManifestMetadataTableNode bug where we were incrementing nextModuleId unnecessarily sometimes. Also added a check that we have finishing sorting the object nodes, since we require that they are sorted to get deterministic output here.
  • Do not flush the ILCache before compiling all methods in a batch. This might have caused the same MethodDesc to have two different MethodIL objects.
  • Add a determinism test that compiles all of coreroot with two different seeds

Anubhav Srivastava added 6 commits November 25, 2019 16:24
@SrivastavaAnubhav SrivastavaAnubhav mentioned this pull request Nov 26, 2019
Closed
@SrivastavaAnubhav
Copy link
Contributor Author

@dotnet/crossgen-contrib

fadimounir
fadimounir suggested changes Nov 27, 2019
View reviewed changes
Copy link
Contributor

@fadimounir fadimounir left a comment

Choose a reason for hiding this comment

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

Looks good, but I have a few suggestions concerning the test

Anubhav Srivastava added 3 commits December 2, 2019 10:59
Address PR comments. Remove batch precommands. Diff all files instead…
532863f 
… of stopping at the first failure. Use CompareExchange for GetLocals and GetExceptionRegions as well.
Check files exist before diff'ing them. Remove output directory if it…
9510b57 
… exists before creating a new one.
Fix environment variable case.
4595444
@fadimounir fadimounir merged commit eb2451c into dotnet:master Dec 3, 2019
@SrivastavaAnubhav SrivastavaAnubhav deleted the determinism-fixes branch December 3, 2019 17:46
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jaredpar jaredpar jaredpar left review comments

@jkotas jkotas jkotas left review comments

@fadimounir fadimounir fadimounir approved these changes

Assignees
No one assigned
Labels
area-crossgen2-coreclr
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SrivastavaAnubhav @jaredpar @jkotas @fadimounir @maryamariyan