Skip to content

Commit 389e7a3

Browse files
dmillerdmiller
committed
CLJCLR-181 - Do not create static fields in function classes for unused constants.
1 parent c7466e3 commit 389e7a3

File tree

4 files changed

+62
-59
lines changed

4 files changed

+62
-59
lines changed

‎Clojure/Clojure/CljCompiler/Ast/ConstantExpr.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public Expr Parse(ParserContext pcon, object form)
115115

116116
public override void Emit(RHC rhc, ObjExpr objx, CljILGen ilg)
117117
{
118-
objx.EmitConstant(ilg, _id, _v);
118+
objx.EmitConstant(ilg, _id);
119119
if (rhc == RHC.Statement)
120120
ilg.Emit(OpCodes.Pop);
121121
}

‎Clojure/Clojure/CljCompiler/Ast/NumberExpr.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ public override object Val
8080
public override void Emit(RHC rhc, ObjExpr objx, CljILGen ilg)
8181
{
8282
if (rhc != RHC.Statement)
83-
objx.EmitConstant(ilg, _id, _n);
83+
objx.EmitConstant(ilg, _id);
84+
8485
}
8586

8687
public bool CanEmitPrimitive => true;

‎Clojure/Clojure/CljCompiler/Ast/ObjExpr.cs‎

Lines changed: 55 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@
99
**/
1010

1111
using clojure.lang.CljCompiler.Context;
12+
1213
using System;
1314
using System.Collections;
1415
using System.Collections.Generic;
15-
using System.Linq;
1616
using System.Reflection;
1717
using System.Reflection.Emit;
1818
using System.Runtime.CompilerServices;
@@ -45,14 +45,26 @@ public class ObjExpr : Expr
4545
public IPersistentMap Keywords { get; internal set; }
4646
public IPersistentMap Vars { get; internal set; }
4747
public IPersistentVector Constants { get; internal set; }
48-
public IPersistentSet UsedConstants { get; } = PersistentHashSet.EMPTY;
4948

50-
public Dictionary<int, FieldBuilder> ConstantFields { get; protected set; }
49+
public class FieldBuilderRecord
50+
{
51+
public FieldBuilderRecord(FieldBuilder fb)
52+
{
53+
FieldBuilder = fb;
54+
Emitted = false;
55+
}
56+
57+
public FieldBuilder FieldBuilder { get; private set; }
58+
public bool Emitted { get; private set; }
59+
public void MarkEmitted() { Emitted = true; }
60+
}
61+
62+
public Dictionary<int, FieldBuilderRecord> ConstantFields { get; protected set; } = [];
5163
public IPersistentMap Fields { get; protected set; } // symbol -> lb
5264
public IPersistentMap SpanMap { get; protected set; }
5365
public Type CompiledType { get; protected set; }
5466
public IPersistentMap ClassMeta { get; protected set; }
55-
public TypeBuilder TypeBuilder { get; protected set; }
67+
public TypeBuilder TypeBuilder { get; internal set; }
5668
public ConstructorInfo CtorInfo { get; protected set; }
5769
public ConstructorInfo BaseClassClosedOverCtor { get; protected set; } // needed by NewInstanceExpr
5870
public ConstructorInfo BaseClassAltCtor { get; protected set; } // needed by NewInstanceExpr
@@ -292,10 +304,8 @@ public Type Compile(Type superType, Type stubType, IPersistentVector interfaces,
292304
//,
293305
//Compiler.COMPILE_STUB_CLASS, _baseType));
294306
}
295-
EmitConstantFieldDefs(TypeBuilder);
296-
EmitKeywordCallsiteDefs(TypeBuilder);
297307

298-
DefineStaticConstructor(TypeBuilder);
308+
EmitKeywordCallsiteDefs(TypeBuilder);
299309

300310
if (SupportsMeta)
301311
MetaField = TypeBuilder.DefineField("__meta", typeof(IPersistentMap), FieldAttributes.Public | FieldAttributes.InitOnly);
@@ -318,6 +328,7 @@ public Type Compile(Type superType, Type stubType, IPersistentVector interfaces,
318328

319329
EmitStatics(TypeBuilder);
320330
EmitMethods(TypeBuilder);
331+
DefineStaticConstructor(TypeBuilder);
321332

322333
CompiledType = TypeBuilder.CreateType();
323334

@@ -425,13 +436,17 @@ private void EmitConstantFieldInits(CljILGen ilg)
425436

426437
for (int i = 0; i < Constants.count(); i++)
427438
{
428-
if (ConstantFields[i] != null)
439+
if (ConstantFields.TryGetValue(i, out FieldBuilderRecord fbr))
429440
{
430-
EmitValue(Constants.nth(i), ilg);
431-
if (Constants.nth(i).GetType() != ConstantType(i))
432-
ilg.Emit(OpCodes.Castclass, ConstantType(i));
433-
FieldBuilder fb = ConstantFields[i];
434-
ilg.Emit(OpCodes.Stsfld, fb);
441+
if (!fbr.Emitted)
442+
{
443+
EmitValue(Constants.nth(i), ilg);
444+
if (Constants.nth(i).GetType() != ConstantType(i))
445+
ilg.Emit(OpCodes.Castclass, ConstantType(i));
446+
//FieldBuilder fb = ConstantFields[i];
447+
ilg.Emit(OpCodes.Stsfld, fbr.FieldBuilder);
448+
fbr.MarkEmitted();
449+
}
435450
}
436451
}
437452
}
@@ -690,32 +705,7 @@ protected virtual void EmitStatics(TypeBuilder tb)
690705
{
691706
}
692707

693-
public void EmitConstantFieldDefs(TypeBuilder baseTB)
694-
{
695-
// We have to do this different than the JVM version.
696-
// The JVM does all these at the end.
697-
// That works for the usual ObjExpr, but not for the top-level one that becomes __Init__.Initialize in the assembly.
698-
// That one need the constants defined incrementally.
699-
// This version accommodates the all-at-end approach for general ObjExprs and the incremental approach in Compiler.Compile1.
700-
701-
if (ConstantFields == null)
702-
ConstantFields = new Dictionary<int, FieldBuilder>(Constants.count());
703-
704-
int nextKey = ConstantFields.Count == 0 ? 0 : ConstantFields.Keys.Max() + 1;
705-
706-
for (int i = nextKey; i < Constants.count(); i++)
707-
{
708-
if (!ConstantFields.ContainsKey(i))
709-
{
710-
string fieldName = ConstantName(i);
711-
Type fieldType = ConstantType(i);
712-
FieldBuilder fb = baseTB.DefineField(fieldName, fieldType, FieldAttributes.FamORAssem | FieldAttributes.Static);
713-
ConstantFields[i] = fb;
714-
}
715-
}
716-
}
717-
718-
public MethodBuilder EmitConstants(TypeBuilder fnTB)
708+
public MethodBuilder DefineConstantFieldInitMethod(TypeBuilder fnTB)
719709
{
720710
try
721711
{
@@ -726,12 +716,17 @@ public MethodBuilder EmitConstants(TypeBuilder fnTB)
726716

727717
for (int i = 0; i < Constants.count(); i++)
728718
{
729-
if (ConstantFields.TryGetValue(i, out FieldBuilder fb))
719+
if (ConstantFields.TryGetValue(i, out FieldBuilderRecord fbr))
730720
{
731-
EmitValue(Constants.nth(i), ilg);
732-
if (Constants.nth(i).GetType() != ConstantType(i))
733-
ilg.Emit(OpCodes.Castclass, ConstantType(i));
734-
ilg.Emit(OpCodes.Stsfld, fb);
721+
if (!fbr.Emitted)
722+
{
723+
EmitValue(Constants.nth(i), ilg);
724+
if (Constants.nth(i).GetType() != ConstantType(i))
725+
ilg.Emit(OpCodes.Castclass, ConstantType(i));
726+
//FieldBuilder fb = ConstantFields[i];
727+
ilg.Emit(OpCodes.Stsfld, fbr.FieldBuilder);
728+
fbr.MarkEmitted();
729+
}
735730
}
736731
}
737732
ilg.Emit(OpCodes.Ret);
@@ -968,15 +963,19 @@ private void EmitListAsObjectArray(object value, CljILGen ilg)
968963
}
969964
}
970965

971-
internal void EmitConstant(CljILGen ilg, int id, object val)
966+
internal void EmitConstant(CljILGen ilg, int id /*, object val */)
972967
{
973-
if (ConstantFields != null && ConstantFields.TryGetValue(id, out FieldBuilder fb))
968+
if (!ConstantFields.TryGetValue(id, out FieldBuilderRecord fbr))
974969
{
975-
ilg.MaybeEmitVolatileOp(fb);
976-
ilg.Emit(OpCodes.Ldsfld, fb);
970+
string fieldName = ConstantName(id);
971+
Type fieldType = ConstantType(id);
972+
FieldBuilder fb = TypeBuilder.DefineField(fieldName, fieldType, FieldAttributes.FamORAssem | FieldAttributes.Static);
973+
ConstantFields[id] = fbr = new FieldBuilderRecord(fb);
977974
}
978-
else
979-
EmitValue(val, ilg);
975+
976+
ilg.MaybeEmitVolatileOp(fbr.FieldBuilder);
977+
ilg.Emit(OpCodes.Ldsfld, fbr.FieldBuilder);
978+
980979
}
981980

982981

@@ -1018,27 +1017,29 @@ internal void EmitConstant(CljILGen ilg, int id, object val)
10181017
internal void EmitVar(CljILGen ilg, Var var)
10191018
{
10201019
int i = (int)Vars.valAt(var);
1021-
EmitConstant(ilg, i, var);
1020+
EmitConstant(ilg, i);
1021+
10221022
}
10231023

10241024

10251025
internal void EmitKeyword(CljILGen ilg, Keyword kw)
10261026
{
10271027
int i = (int)Keywords.valAt(kw);
1028-
EmitConstant(ilg, i, kw);
1028+
EmitConstant(ilg, i);
1029+
10291030
}
10301031

10311032
internal void EmitVarValue(CljILGen ilg, Var v)
10321033
{
10331034
int i = (int)Vars.valAt(v);
10341035
if (!v.isDynamic())
10351036
{
1036-
EmitConstant(ilg, i, v);
1037+
EmitConstant(ilg, i);
10371038
ilg.Emit(OpCodes.Call, Compiler.Method_Var_getRawRoot);
10381039
}
10391040
else
10401041
{
1041-
EmitConstant(ilg, i, v);
1042+
EmitConstant(ilg, i);
10421043
ilg.Emit(OpCodes.Call, Compiler.Method_Var_get); // or just Method_Var_get??
10431044
}
10441045
}

‎Clojure/Clojure/CljCompiler/Compiler.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,6 +1740,7 @@ public static object Compile(GenContext context, TextReader rdr, string sourceDi
17401740

17411741
TypeBuilder initTB = context.AssemblyGen.DefinePublicType(InitClassName(internalName), typeof(object), true);
17421742
context = context.WithTypeBuilder(initTB);
1743+
objx.TypeBuilder = initTB;
17431744

17441745
// static load method
17451746
MethodBuilder initMB = initTB.DefineMethod("Initialize", MethodAttributes.Public | MethodAttributes.Static, typeof(void), Type.EmptyTypes);
@@ -1786,8 +1787,8 @@ public static object Compile(GenContext context, TextReader rdr, string sourceDi
17861787
initMB.GetILGenerator().Emit(OpCodes.Ret);
17871788

17881789
// static fields for constants
1789-
objx.EmitConstantFieldDefs(initTB);
1790-
MethodBuilder constInitsMB = objx.EmitConstants(initTB);
1790+
//objx.EmitConstantFieldDefs(initTB);
1791+
MethodBuilder constInitsMB = objx.DefineConstantFieldInitMethod(initTB);
17911792

17921793
// Static init for constants, keywords, vars
17931794
ConstructorBuilder cb = initTB.DefineConstructor(MethodAttributes.Static, CallingConventions.Standard, Type.EmptyTypes);
@@ -1853,7 +1854,7 @@ private static void Compile1(TypeBuilder tb, CljILGen ilg, ObjExpr objx, object
18531854
objx.Keywords = (IPersistentMap)KeywordsVar.deref();
18541855
objx.Vars = (IPersistentMap)VarsVar.deref();
18551856
objx.Constants = (PersistentVector)ConstantsVar.deref();
1856-
objx.EmitConstantFieldDefs(tb);
1857+
//objx.EmitConstantFieldDefs(tb);
18571858
expr.Emit(RHC.Expression, objx, ilg);
18581859
ilg.Emit(OpCodes.Pop);
18591860

0 commit comments

Comments
 (0)