Skip to content
This repository was archived by the owner on Jul 6, 2026. It is now read-only.

[generator] Enable parallel type generation. - #447

Merged
jonpryor merged 1 commit into
masterfrom
parallel-types-2
Jul 18, 2019
Merged

[generator] Enable parallel type generation.#447
jonpryor merged 1 commit into
masterfrom
parallel-types-2

Conversation

@jpobst

@jpobstjpobst commented Jul 18, 2019

Copy link
Copy Markdown
Contributor

#446 removed Cecil usage from the step that generates the type .cs files, which was the remaining thread-safety issue. This PR enables generating those files in parallel.

This results in a nice performance win, particularly for projects binding a lot of types:

ScenarioOriginalThis PR
Mono.Android.dll16423ms11506ms
Jar2Xml5517ms5309ms
Class-Parse15511ms15615ms

@jonpryor

jonpryor commented Jul 18, 2019

Copy link
Copy Markdown
Contributor

Odd that class-parse got slower...

@jonpryor

jonpryor commented Jul 18, 2019

Copy link
Copy Markdown
Contributor

What have you done in terms of "stress testing" to ensure that PR #446 didn't "miss" anything? Have you tried running generator.exe multiple times into separate output directories, and diff the generated outputs to ensure consistency?

@jpobst

jpobst commented Jul 18, 2019•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

I shouldn't have included class-parse results for this PR, as that test case does not write types. (it uses --only-xml-adjuster) It's just benchmark margin of error. ;)

My testing is roughly what you suggest. I committed the "known good" generated output to a local git repository. I then run generator.exe into a new directory, robocopy /purge it to the git directory (so it would remove any missing files), and then git diff it to ensure nothing has changed.

@jonpryor
jonpryor merged commit 2ec06c9 into masterJul 18, 2019
@jpobst
jpobst deleted the parallel-types-2 branch July 19, 2019 15:56
jpobst added a commit that referenced this pull request Jul 25, 2019
@jpobst
Revert "[generator] Enable parallel type generation (#447)"
145841d
This reverts commit 2ec06c9.
@jpobstjpobst mentioned this pull request Jul 25, 2019
Merged
jonpryor pushed a commit that referenced this pull request Jul 26, 2019
@jpobst@jonpryor
Revert "[generator] Enable parallel type generation (#447)" (#453)
4e7520a
Reverts commit 2ec06c9.
Revert parallel type generation. There are some deeper thread-safety
issues that will have to be resolved before this can safely be
enabled. These take the form of `CodeGenerator` manipulating the
object model while it is writing, e.g. within
[`CodeGenerator.WriteProperty()`][0]:
string pname = property.Setter.Parameters [0].Name;
property.Setter.Parameters [0].Name = "value";
WriteMethodBody (property.Setter, indent + "\t\t");
property.Setter.Parameters [0].Name = pname;
This caused a test to sporadically fail on CI, but the pipeline is
hiding test errors so it wasn't noticed.
[0]: https://github.com/xamarin/java.interop/blob/master/tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs#L1437-L1440
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 13, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jpobst@jonpryor