Skip to content

Commit 2ea8f7d

Browse files
jpobstjonpryor
jpobst
authored and
jonpryor
committed
[generator] Remove global state around code generation (#444)
Review global state use: * Don't use `CodeGenerationOptions` to store generation context state, use the new `CodeGeneratorContext` instead. * Don't store a global `StreamWriter` and ensure `StreamWriter`s are disposed. * Remove some shared state from `GenerationInfo`. Once we have a reasonable grasp on our global state use, we can better investigate the introduction of thread parallel algorithms.
1 parent 4b2c6a2 commit 2ea8f7d

39 files changed

+372
-421
lines changed

‎tools/generator/CodeGenerationOptions.cs‎

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Concurrent;
23
using System.Collections.Generic;
34
using System.IO;
45
using System.Linq;
@@ -10,29 +11,6 @@ namespace MonoDroid.Generation
1011
{
1112
public class CodeGenerationOptions
1213
{
13-
Stack<GenBase> context_types = new Stack<GenBase> ();
14-
public Stack<GenBase> ContextTypes {
15-
get { return context_types; }
16-
}
17-
public List<Method> ContextGeneratedMethods { get; set; } = new List<Method> ();
18-
public GenBase ContextType {
19-
get { return context_types.Any () ? context_types.Peek () : null; }
20-
}
21-
public Field ContextField { get; set; }
22-
string ContextFieldString {
23-
get { return ContextField != null ? "in field " + ContextField.Name + " " : null; }
24-
}
25-
public MethodBase ContextMethod { get; set; }
26-
string ContextMethodString {
27-
get { return ContextMethod != null ? "in method " + ContextMethod.Name + " " : null; }
28-
}
29-
string ContextTypeString {
30-
get { return ContextType != null ? "in managed type " + ContextType.FullName : null; }
31-
}
32-
public string ContextString {
33-
get { return ContextFieldString + ContextMethodString + ContextTypeString; }
34-
}
35-
3614
CodeGenerationTarget codeGenerationTarget;
3715
public CodeGenerationTarget CodeGenerationTarget {
3816
get { return codeGenerationTarget; }
@@ -134,20 +112,23 @@ public string GetSafeIdentifier (string name)
134112
return name.Replace ('$', '_');
135113
}
136114

137-
Dictionary<string,string> short_file_names = new Dictionary<string, string> ();
115+
readonly Dictionary<string,string> short_file_names = new Dictionary<string, string> ();
138116

139117
public string GetFileName (string fullName)
140118
{
141119
if (!UseShortFileNames)
142120
return fullName;
143-
string s;
144-
if (short_file_names.TryGetValue (fullName, out s))
121+
122+
lock (short_file_names) {
123+
if (short_file_names.TryGetValue (fullName, out var s))
124+
return s;
125+
126+
s = short_file_names.Count.ToString ();
127+
short_file_names [fullName] = s;
128+
145129
return s;
146-
s = short_file_names.Count.ToString ();
147-
short_file_names [fullName] = s;
148-
return s;
130+
}
149131
}
150132
}
151-
152133
}
153134

‎tools/generator/CodeGenerator.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ static void Run (CodeGeneratorOptions options, DirectoryAssemblyResolver resolve
140140
AddTypeToTable (opt, gen);
141141
}
142142

143-
Validate (gens, opt);
143+
Validate (gens, opt, new CodeGeneratorContext ());
144144

145145
if (api_versions_xml != null)
146146
ApiVersionsSupport.AssignApiLevels (gens, api_versions_xml);
@@ -175,6 +175,7 @@ static void Run (CodeGeneratorOptions options, DirectoryAssemblyResolver resolve
175175
if (gen.IsGeneratable)
176176
gen.Generate (opt, gen_info);
177177

178+
178179
ClassGen.GenerateTypeRegistrations (opt, gen_info);
179180
ClassGen.GenerateEnumList (gen_info);
180181

@@ -211,7 +212,7 @@ static IEnumerable<GenBase> FlattenNestedTypes (IEnumerable<GenBase> gens)
211212
}
212213
}
213214

214-
static void Validate (List<GenBase> gens, CodeGenerationOptions opt)
215+
static void Validate (List<GenBase> gens, CodeGenerationOptions opt, CodeGeneratorContext context)
215216
{
216217
//int cycle = 1;
217218
List<GenBase> removed = new List<GenBase> ();
@@ -229,7 +230,7 @@ static void Validate (List<GenBase> gens, CodeGenerationOptions opt)
229230
foreach (GenBase gen in gens)
230231
if ((opt.IgnoreNonPublicType &&
231232
(gen.RawVisibility != "public" && gen.RawVisibility != "internal"))
232-
|| !gen.Validate (opt, null)) {
233+
|| !gen.Validate (opt, null, context)) {
233234
foreach (GenBase nest in gen.NestedTypes) {
234235
foreach (var nt in nest.Invalidate ())
235236
removed.Add (nt);

‎tools/generator/CodeGeneratorContext.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
5+
namespace MonoDroid.Generation
6+
{
7+
public class CodeGeneratorContext
8+
{
9+
public Stack<GenBase> ContextTypes { get; } = new Stack<GenBase> ();
10+
public List<Method> ContextGeneratedMethods { get; set; } = new List<Method> ();
11+
public Field ContextField { get; set; }
12+
public MethodBase ContextMethod { get; set; }
13+
14+
public GenBase ContextType => ContextTypes.Any () ? ContextTypes.Peek () : null;
15+
string ContextFieldString => ContextField != null ? "in field " + ContextField.Name + " " : null;
16+
string ContextMethodString => ContextMethod != null ? "in method " + ContextMethod.Name + " " : null;
17+
string ContextTypeString => ContextType != null ? "in managed type " + ContextType.FullName : null;
18+
public string ContextString => ContextFieldString + ContextMethodString + ContextTypeString;
19+
}
20+
}

‎tools/generator/GenerationInfo.cs‎

Lines changed: 16 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Concurrent;
23
using System.Collections.Generic;
34
using System.IO;
45
using System.Linq;
@@ -13,77 +14,36 @@ public class GenerationInfo {
1314

1415
public GenerationInfo (string csdir, string javadir, string assembly)
1516
{
16-
this.csdir = csdir;
17-
this.javadir = javadir;
18-
this.assembly = assembly;
17+
CSharpDir = csdir;
18+
JavaDir = javadir;
19+
Assembly = assembly;
1920
}
2021

21-
string assembly;
22-
public string Assembly {
23-
get { return assembly; }
24-
}
25-
26-
string csdir;
27-
public string CSharpDir {
28-
get { return csdir; }
29-
}
30-
31-
string javadir;
32-
public string JavaDir {
33-
get { return javadir; }
34-
}
35-
36-
string member;
37-
public string CurrentMember {
38-
get { return typename + "." + member; }
39-
set { member = value; }
40-
}
41-
42-
string typename;
43-
public string CurrentType {
44-
get { return typename; }
45-
set { typename = value; }
46-
}
47-
48-
StreamWriter sw;
49-
public StreamWriter Writer {
50-
get { return sw; }
51-
set { sw = value; }
52-
}
53-
54-
List<string> generated_files = new List<string> ();
55-
public IEnumerable<string> GeneratedFiles {
56-
get { return generated_files; }
57-
}
22+
public string Assembly { get; }
23+
public string CSharpDir { get; }
24+
public string JavaDir { get; }
25+
public ConcurrentBag<string> GeneratedFiles { get; } = new ConcurrentBag<string> ();
26+
public ConcurrentBag<string> Enums { get; } = new ConcurrentBag<string> ();
27+
public ConcurrentBag<KeyValuePair<string, string>> TypeRegistrations { get; } = new ConcurrentBag<KeyValuePair<string, string>> ();
5828

5929
public StreamWriter OpenStream (string name)
6030
{
61-
if (!Directory.Exists(csdir))
62-
Directory.CreateDirectory (csdir);
63-
string filename = Path.Combine (csdir, name + ".cs");
31+
if (!Directory.Exists (CSharpDir))
32+
Directory.CreateDirectory (CSharpDir);
33+
string filename = Path.Combine (CSharpDir, name + ".cs");
6434

65-
sw = new StreamWriter (File.Create (filename));
66-
generated_files.Add (filename);
35+
var sw = new StreamWriter (File.Create (filename));
36+
GeneratedFiles.Add (filename);
6737
return sw;
6838
}
6939

70-
List<string> enums = new List<string> ();
71-
public ICollection<string> Enums {
72-
get { return enums; }
73-
}
74-
75-
List<KeyValuePair<string, string>> type_registrations = new List<KeyValuePair<string, string>> ();
76-
public ICollection<KeyValuePair<string, string>> TypeRegistrations {
77-
get { return type_registrations; }
78-
}
79-
8040
internal void GenerateLibraryProjectFile (CodeGeneratorOptions options, IEnumerable<string> enumFiles, string path = null)
8141
{
8242
if (path == null) {
8343
var name = Assembly ?? "GeneratedFiles";
8444
int idx = name.IndexOf (',');
8545
name = idx < 0 ? name : name.Substring (0, idx);
86-
path = Path.Combine (csdir, name + ".projitems");
46+
path = Path.Combine (CSharpDir, name + ".projitems");
8747
}
8848

8949
var msbuild = XNamespace.Get ("http://schemas.microsoft.com/developer/msbuild/2003");

‎tools/generator/Java.Interop.Tools.Generator.CodeGeneration/CodeGenerator.cs‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections;
23
using System.Collections.Generic;
34
using System.Collections.Specialized;
45
using System.IO;
@@ -13,6 +14,8 @@ abstract class CodeGenerator
1314
protected TextWriter writer;
1415
protected CodeGenerationOptions opt;
1516

17+
public CodeGeneratorContext Context { get; } = new CodeGeneratorContext ();
18+
1619
protected CodeGenerator (TextWriter writer, CodeGenerationOptions options)
1720
{
1821
this.writer = writer;
@@ -38,8 +41,8 @@ protected CodeGenerator (TextWriter writer, CodeGenerationOptions options)
3841

3942
public void WriteClass (ClassGen @class, string indent, GenerationInfo gen_info)
4043
{
41-
opt.ContextTypes.Push (@class);
42-
opt.ContextGeneratedMethods = new List<Method> ();
44+
Context.ContextTypes.Push (@class);
45+
Context.ContextGeneratedMethods = new List<Method> ();
4346

4447
gen_info.TypeRegistrations.Add (new KeyValuePair<string, string> (@class.RawJniName, @class.AssemblyQualifiedName));
4548
bool is_enum = @class.base_symbol != null && @class.base_symbol.FullName == "Java.Lang.Enum";
@@ -186,9 +189,9 @@ public void WriteClass (ClassGen @class, string indent, GenerationInfo gen_info)
186189
WriteClassInvoker (@class, indent);
187190
}
188191

189-
opt.ContextGeneratedMethods.Clear ();
192+
Context.ContextGeneratedMethods.Clear ();
190193

191-
opt.ContextTypes.Pop ();
194+
Context.ContextTypes.Pop ();
192195
}
193196

194197
public void WriteClassAbstractMembers (ClassGen @class, string indent)
@@ -289,7 +292,7 @@ public void WriteClassMethods (ClassGen @class, string indent)
289292
WriteMethodAbstractDeclaration (m, indent, null, @class);
290293
else
291294
WriteMethod (m, indent, @class, true);
292-
opt.ContextGeneratedMethods.Add (m);
295+
Context.ContextGeneratedMethods.Add (m);
293296
m.IsVirtual = virt;
294297
}
295298

@@ -393,7 +396,7 @@ public bool WriteFields (List<Field> fields, string indent, GenBase gen, HashSet
393396
Report.Warning (0, Report.WarningDuplicateField, "Skipping {0}.{1}, due to a duplicate field. (Field) (Java type: {2})", gen.FullName, f.Name, gen.JavaName);
394397
continue;
395398
}
396-
if (f.Validate (opt, gen.TypeParameters)) {
399+
if (f.Validate (opt, gen.TypeParameters, Context)) {
397400
if (seen != null)
398401
seen.Add (f.Name);
399402
needsProperty = needsProperty || f.NeedsProperty;
@@ -440,7 +443,7 @@ internal virtual void WriteField (Field field, string indent, GenBase type)
440443

441444
public void WriteInterface (InterfaceGen @interface, string indent, GenerationInfo gen_info)
442445
{
443-
opt.ContextTypes.Push (@interface);
446+
Context.ContextTypes.Push (@interface);
444447

445448
// interfaces don't nest, so generate as siblings
446449
foreach (GenBase nest in @interface.NestedTypes) {
@@ -465,7 +468,7 @@ public void WriteInterface (InterfaceGen @interface, string indent, GenerationIn
465468
WriteInterfaceExtensionsDeclaration (@interface, indent, null);
466469
WriteInterfaceInvoker (@interface, indent);
467470
WriteInterfaceEventHandler (@interface, indent);
468-
opt.ContextTypes.Pop ();
471+
Context.ContextTypes.Pop ();
469472
}
470473

471474
// For each interface, generate either an abstract method or an explicit implementation method.
@@ -474,7 +477,7 @@ public void WriteInterfaceAbstractMembers (InterfaceGen @interface, ClassGen gen
474477
foreach (Method m in @interface.Methods.Where (m => !m.IsInterfaceDefaultMethod && !m.IsStatic)) {
475478
bool mapped = false;
476479
string sig = m.GetSignature ();
477-
if (opt.ContextGeneratedMethods.Any (_ => _.Name == m.Name && _.JniSignature == m.JniSignature))
480+
if (Context.ContextGeneratedMethods.Any (_ => _.Name == m.Name && _.JniSignature == m.JniSignature))
478481
continue;
479482
for (var cls = gen; cls != null; cls = cls.BaseGen)
480483
if (cls.ContainsMethod (m, false) || cls != gen && gen.ExplicitlyImplementedInterfaceMethods.Contains (sig)) {
@@ -487,7 +490,7 @@ public void WriteInterfaceAbstractMembers (InterfaceGen @interface, ClassGen gen
487490
WriteMethodExplicitInterfaceImplementation (m, indent, @interface);
488491
else
489492
WriteMethodAbstractDeclaration (m, indent, @interface, gen);
490-
opt.ContextGeneratedMethods.Add (m);
493+
Context.ContextGeneratedMethods.Add (m);
491494
}
492495
foreach (Property prop in @interface.Properties.Where (p => !p.Getter.IsStatic)) {
493496
if (gen.ContainsProperty (prop.Name, false))
@@ -777,7 +780,7 @@ public void WriteInterfaceInvoker (InterfaceGen @interface, string indent)
777780
writer.WriteLine ();
778781
writer.WriteLine ("{0}\tpublic {1}Invoker (IntPtr handle, JniHandleOwnership transfer) : base (Validate (handle), transfer)", indent, @interface.Name);
779782
writer.WriteLine ("{0}\t{{", indent);
780-
writer.WriteLine ("{0}\t\tIntPtr local_ref = JNIEnv.GetObjectClass ({1});", indent, opt.ContextType.GetObjectHandleProperty ("this"));
783+
writer.WriteLine ("{0}\t\tIntPtr local_ref = JNIEnv.GetObjectClass ({1});", indent, Context.ContextType.GetObjectHandleProperty ("this"));
781784
writer.WriteLine ("{0}\t\tthis.class_ref = JNIEnv.NewGlobalRef (local_ref);", indent);
782785
writer.WriteLine ("{0}\t\tJNIEnv.DeleteLocalRef (local_ref);", indent);
783786
writer.WriteLine ("{0}\t}}", indent);
@@ -1176,7 +1179,7 @@ public void WriteMethodInvokerBody (Method method, string indent)
11761179
WriteParameterListCallArgs (method.Parameters, indent, invoker: true);
11771180
string env_method = "Call" + method.RetVal.CallMethodPrefix + "Method";
11781181
string call = "JNIEnv." + env_method + " (" +
1179-
opt.ContextType.GetObjectHandleProperty ("this") + ", " + method.EscapedIdName + method.Parameters.GetCallArgs (opt, invoker: true) + ")";
1182+
Context.ContextType.GetObjectHandleProperty ("this") + ", " + method.EscapedIdName + method.Parameters.GetCallArgs (opt, invoker: true) + ")";
11801183
if (method.IsVoid)
11811184
writer.WriteLine ("{0}{1};", indent, call);
11821185
else

‎tools/generator/Java.Interop.Tools.Generator.CodeGeneration/JavaInteropCodeGenerator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ internal override void WriteConstructorBody (Ctor ctor, string indent, System.Co
120120
? "(" + ctor.Parameters.JniNestedDerivedSignature + ")V"
121121
: ctor.JniSignature);
122122
writer.WriteLine ();
123-
writer.WriteLine ("{0}if ({1} != IntPtr.Zero)", indent, opt.ContextType.GetObjectHandleProperty ("this"));
123+
writer.WriteLine ("{0}if ({1} != IntPtr.Zero)", indent, Context.ContextType.GetObjectHandleProperty ("this"));
124124
writer.WriteLine ("{0}\treturn;", indent);
125125
writer.WriteLine ();
126126
foreach (string prep in ctor.Parameters.GetCallPrep (opt))

‎tools/generator/Java.Interop.Tools.Generator.CodeGeneration/XamarinAndroidCodeGenerator.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.ComponentModel.Design;
23
using System.IO;
34

45
namespace MonoDroid.Generation {
@@ -65,7 +66,7 @@ internal override void WriteConstructorIdField (Ctor ctor, string indent)
6566

6667
internal override void WriteConstructorBody (Ctor ctor, string indent, System.Collections.Specialized.StringCollection call_cleanup)
6768
{
68-
writer.WriteLine ("{0}if ({1} != IntPtr.Zero)", indent, opt.ContextType.GetObjectHandleProperty ("this"));
69+
writer.WriteLine ("{0}if ({1} != IntPtr.Zero)", indent, Context.ContextType.GetObjectHandleProperty ("this"));
6970
writer.WriteLine ("{0}\treturn;", indent);
7071
writer.WriteLine ();
7172
foreach (string prep in ctor.Parameters.GetCallPrep (opt))
@@ -83,7 +84,7 @@ internal override void WriteConstructorBody (Ctor ctor, string indent, System.Co
8384
writer.WriteLine ("{0}\t\t\tJniHandleOwnership.TransferLocalRef);", indent);
8485
writer.WriteLine ("{0}\tglobal::Android.Runtime.JNIEnv.FinishCreateInstance ({1}, \"{2}\"{3});",
8586
indent,
86-
opt.ContextType.GetObjectHandleProperty ("this"),
87+
Context.ContextType.GetObjectHandleProperty ("this"),
8788
ctor.IsNonStaticNestedType ? "(" + ctor.Parameters.JniNestedDerivedSignature + ")V" : ctor.JniSignature,
8889
ctor.Parameters.GetCallArgs (opt, invoker:false));
8990
writer.WriteLine ("{0}\treturn;", indent);
@@ -97,7 +98,7 @@ internal override void WriteConstructorBody (Ctor ctor, string indent, System.Co
9798
writer.WriteLine ("{0}\t\tJniHandleOwnership.TransferLocalRef);", indent);
9899
writer.WriteLine ("{0}JNIEnv.FinishCreateInstance ({1}, class_ref, {2}{3});",
99100
indent,
100-
opt.ContextType.GetObjectHandleProperty ("this"),
101+
Context.ContextType.GetObjectHandleProperty ("this"),
101102
ctor.ID,
102103
ctor.Parameters.GetCallArgs (opt, invoker:false));
103104
indent = oldindent;

‎tools/generator/Java.Interop.Tools.Generator.Importers/Xml/XmlCtor.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,11 @@ public override string Name {
6868
set { name = value; }
6969
}
7070

71-
protected override bool OnValidate (CodeGenerationOptions opt, GenericParameterDefinitionList tps)
71+
protected override bool OnValidate (CodeGenerationOptions opt, GenericParameterDefinitionList tps, CodeGeneratorContext context)
7272
{
7373
if (missing_enclosing_class)
7474
return false;
75-
return base.OnValidate (opt, tps);
75+
return base.OnValidate (opt, tps, context);
7676
}
7777

7878
public override string CustomAttributes {

0 commit comments

Comments
 (0)