-
Notifications
You must be signed in to change notification settings - Fork 59
[generator] Encapsulate and parallelize enum generation. #442
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
tools/generator/Java.Interop.Tools.Generator.CodeGeneration/EnumGenerator.cs
Outdated
Show resolved
Hide resolved
tools/generator/Java.Interop.Tools.Generator.Transformation/EnumMappings.cs
Outdated
Show resolved
Hide resolved
return cls.FullName + "." + fld.Name; | ||
}); | ||
|
||
return files.ToList (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that this should be files.OrderBy (f => f).ToList ()
, or some variation thereof.
Rationale: C# deterministic builds. If, for whatever reason, generator
is re-executed during a build, it would be nice if, when deterministic builds are enabled, the resulting assembly did not change unless the generated code actually changed.
The order of generated files appears to be used in the csc /deterministic
algorithm, and thus altering the order of files may result in the creation of a different assembly. For example:
// app1.cs
class App1 {
public static void Main ()
{
App2 a = new App2 ();
}
}
// app2.cs
class App2 {
}
$ csc /deterministic /out:upwards.exe app1.cs app2.cs
$ csc /deterministic /out:downwards.exe app2.cs app1.cs
$ md5 downwards.exe upwards.exe
MD5 (downwards.exe) = cb7690e4878515303bde517447130b28
MD5 (upwards.exe) = b258bd49fe38709db5191f1adef9afc9
"Simply" changing the order of command-line arguments results in a different assembly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do the sort when we write the project file:
https://github.com/xamarin/java.interop/blob/master/tools/generator/GenerationInfo.cs#L64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I missed that. Thanks!
KeyValuePair<string, EnumMappings.EnumDescription> CreateEnum () | ||
{ | ||
var enu = new EnumMappings.EnumDescription { | ||
Members = new Dictionary<string, string> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entirely unrelated, this Members
and JniNames
APIs are ugly; they need to be consistent with each other, but they're entirely separate properties, and thus need not actually be consistent. :-(
(This doesn't need to be fixed here; it's simply that I noticed this now.)
Encapsulate the generation of the enum files to
EnumGenerator
. This helps make it testable and easier to ensure it is thread-safe. With it being thread-safe we can now generate these files in parallel. On my 6 core machine, this reduces the time taken from ~5.5s to ~1.1s.