Skip to content

Commit 82e736f

Browse files
kgflibitijibibo
andauthored
Reduce EffectParameter-related heap allocations (#434)
- Don't allocate unique EffectAnnotationCollections if there will be no items in them - For EffectPassCollection instances containing a single item, don't allocate a backing list (unless needed for GetEnumerator) - Lazily allocate the EffectParameter.Elements and EffectParameter.StructureMembers collections - Expand editorconfig to accomodate formatting in Effect implementation Co-authored-by: Ethan Lee <[email protected]>
1 parent 9fce07f commit 82e736f

File tree

5 files changed

+225
-69
lines changed

5 files changed

+225
-69
lines changed

.editorconfig

+7
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,10 @@
44
indent_style = tab
55
trim_trailing_whitespace = true
66
insert_final_newline = true
7+
8+
csharp_new_line_before_open_brace = all
9+
csharp_new_line_before_else = true
10+
csharp_new_line_before_catch = true
11+
csharp_new_line_before_finally = true
12+
csharp_space_after_cast = true
13+
csharp_space_between_method_declaration_name_and_open_parenthesis = false

src/Graphics/Effect/Effect.cs

+83-48
Original file line numberDiff line numberDiff line change
@@ -925,40 +925,6 @@ private unsafe void INTERNAL_parseEffectStruct(IntPtr effectData)
925925
continue;
926926
}
927927

928-
EffectParameterCollection structMembers = null;
929-
if (param.value.type.member_count > 0)
930-
{
931-
List<EffectParameter> memList = new List<EffectParameter>();
932-
unsafe
933-
{
934-
MOJOSHADER_symbolStructMember* mem = (MOJOSHADER_symbolStructMember*) param.value.type.members;
935-
IntPtr curOffset = IntPtr.Zero;
936-
for (int j = 0; j < param.value.type.member_count; j += 1)
937-
{
938-
uint memSize = mem[j].info.rows * mem[j].info.columns;
939-
if (mem[j].info.elements > 0)
940-
{
941-
memSize *= mem[j].info.elements;
942-
}
943-
memList.Add(new EffectParameter(
944-
Marshal.PtrToStringAnsi(mem[j].name),
945-
null,
946-
(int) mem[j].info.rows,
947-
(int) mem[j].info.columns,
948-
(int) mem[j].info.elements,
949-
XNAClass[(int) mem[j].info.parameter_class],
950-
XNAType[(int) mem[j].info.parameter_type],
951-
null, // FIXME: Nested structs! -flibit
952-
null,
953-
param.value.values + curOffset.ToInt32(),
954-
memSize * 4
955-
));
956-
curOffset += (int) memSize * 4;
957-
}
958-
}
959-
structMembers = new EffectParameterCollection(memList);
960-
}
961-
962928
parameters.Add(new EffectParameter(
963929
Marshal.PtrToStringAnsi(param.value.name),
964930
Marshal.PtrToStringAnsi(param.value.semantic),
@@ -967,7 +933,7 @@ private unsafe void INTERNAL_parseEffectStruct(IntPtr effectData)
967933
(int) param.value.type.elements,
968934
XNAClass[(int) param.value.type.parameter_class],
969935
XNAType[(int) param.value.type.parameter_type],
970-
structMembers,
936+
new IntPtr(&paramPtr[i].value.type),
971937
INTERNAL_readAnnotations(
972938
param.annotations,
973939
param.annotation_count
@@ -985,26 +951,27 @@ private unsafe void INTERNAL_parseEffectStruct(IntPtr effectData)
985951
{
986952
// Set up Passes
987953
MOJOSHADER_effectPass* passPtr = (MOJOSHADER_effectPass*) techPtr->passes;
988-
List<EffectPass> passes = new List<EffectPass>((int) techPtr->pass_count);
989-
for (int j = 0; j < passes.Capacity; j += 1)
954+
EffectPassCollection passes;
955+
if (techPtr->pass_count == 1)
990956
{
991-
MOJOSHADER_effectPass pass = passPtr[j];
992-
passes.Add(new EffectPass(
993-
Marshal.PtrToStringAnsi(pass.name),
994-
INTERNAL_readAnnotations(
995-
pass.annotations,
996-
pass.annotation_count
997-
),
998-
this,
999-
(IntPtr) techPtr,
1000-
(uint) j
957+
passes = new EffectPassCollection(INTERNAL_readPass(
958+
ref passPtr[0], (IntPtr) techPtr, 0
1001959
));
1002960
}
961+
else
962+
{
963+
List<EffectPass> passList = new List<EffectPass>((int) techPtr->pass_count);
964+
for (int j = 0; j < passList.Capacity; j += 1)
965+
{
966+
passList.Add(INTERNAL_readPass(ref passPtr[j], (IntPtr) techPtr, (uint) j));
967+
}
968+
passes = new EffectPassCollection(passList);
969+
}
1003970

1004971
techniques.Add(new EffectTechnique(
1005972
Marshal.PtrToStringAnsi(techPtr->name),
1006973
(IntPtr) techPtr,
1007-
new EffectPassCollection(passes),
974+
passes,
1008975
INTERNAL_readAnnotations(
1009976
techPtr->annotations,
1010977
techPtr->annotation_count
@@ -1014,10 +981,78 @@ private unsafe void INTERNAL_parseEffectStruct(IntPtr effectData)
1014981
Techniques = new EffectTechniqueCollection(techniques);
1015982
}
1016983

984+
internal unsafe static EffectParameterCollection INTERNAL_readEffectParameterStructureMembers(
985+
EffectParameter parameter,
986+
IntPtr _type
987+
) {
988+
if (_type == IntPtr.Zero)
989+
{
990+
return null;
991+
}
992+
993+
var type = *(MOJOSHADER_symbolTypeInfo*) _type;
994+
EffectParameterCollection structMembers = null;
995+
if (type.member_count > 0)
996+
{
997+
List<EffectParameter> memList = new List<EffectParameter>();
998+
unsafe
999+
{
1000+
MOJOSHADER_symbolStructMember* mem = (MOJOSHADER_symbolStructMember*) type.members;
1001+
IntPtr curOffset = IntPtr.Zero;
1002+
for (int j = 0; j < type.member_count; j += 1)
1003+
{
1004+
uint memSize = mem[j].info.rows * mem[j].info.columns;
1005+
if (mem[j].info.elements > 0)
1006+
{
1007+
memSize *= mem[j].info.elements;
1008+
}
1009+
memList.Add(new EffectParameter(
1010+
Marshal.PtrToStringAnsi(mem[j].name),
1011+
null,
1012+
(int) mem[j].info.rows,
1013+
(int) mem[j].info.columns,
1014+
(int) mem[j].info.elements,
1015+
XNAClass[(int) mem[j].info.parameter_class],
1016+
XNAType[(int) mem[j].info.parameter_type],
1017+
IntPtr.Zero, // FIXME: Nested structs! -flibit
1018+
null,
1019+
parameter.values + curOffset.ToInt32(),
1020+
memSize * 4
1021+
));
1022+
curOffset += (int) memSize * 4;
1023+
}
1024+
}
1025+
structMembers = new EffectParameterCollection(memList);
1026+
}
1027+
1028+
return structMembers;
1029+
}
1030+
1031+
private unsafe EffectPass INTERNAL_readPass(
1032+
ref MOJOSHADER_effectPass pass,
1033+
IntPtr techPtr, uint index
1034+
) {
1035+
return new EffectPass(
1036+
Marshal.PtrToStringAnsi(pass.name),
1037+
INTERNAL_readAnnotations(
1038+
pass.annotations,
1039+
pass.annotation_count
1040+
),
1041+
this,
1042+
techPtr,
1043+
index
1044+
);
1045+
}
1046+
10171047
private unsafe EffectAnnotationCollection INTERNAL_readAnnotations(
10181048
IntPtr rawAnnotations,
10191049
uint numAnnotations
10201050
) {
1051+
if (numAnnotations == 0)
1052+
{
1053+
return EffectAnnotationCollection.Empty;
1054+
}
1055+
10211056
MOJOSHADER_effectAnnotation* annoPtr = (MOJOSHADER_effectAnnotation*) rawAnnotations;
10221057
List<EffectAnnotation> annotations = new List<EffectAnnotation>((int) numAnnotations);
10231058
for (int i = 0; i < numAnnotations; i += 1)

src/Graphics/Effect/EffectAnnotationCollection.cs

+7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ namespace Microsoft.Xna.Framework.Graphics
1616
{
1717
public sealed class EffectAnnotationCollection : IEnumerable<EffectAnnotation>, IEnumerable
1818
{
19+
#region Allocation optimization
20+
21+
internal static readonly EffectAnnotationCollection Empty =
22+
new EffectAnnotationCollection(new List<EffectAnnotation>());
23+
24+
#endregion
25+
1926
#region Public Properties
2027

2128
public int Count

src/Graphics/Effect/EffectParameter.cs

+81-17
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,26 @@ public EffectParameterType ParameterType
5757

5858
public EffectParameterCollection Elements
5959
{
60-
get;
61-
private set;
60+
get
61+
{
62+
if ((elementCount > 0) && (elements == null))
63+
{
64+
BuildElementList();
65+
}
66+
return elements;
67+
}
6268
}
6369

6470
public EffectParameterCollection StructureMembers
6571
{
66-
get;
67-
private set;
72+
get
73+
{
74+
if ((mojoType != IntPtr.Zero) && (members == null))
75+
{
76+
BuildMemberList();
77+
}
78+
return members;
79+
}
6880
}
6981

7082
public EffectAnnotationCollection Annotations
@@ -82,10 +94,47 @@ public EffectAnnotationCollection Annotations
8294
internal IntPtr values;
8395
internal uint valuesSizeBytes;
8496

97+
internal IntPtr mojoType;
98+
99+
internal int elementCount;
100+
internal EffectParameterCollection elements;
101+
internal EffectParameterCollection members;
102+
85103
#endregion
86104

87105
#region Internal Constructor
88106

107+
internal EffectParameter(
108+
string name,
109+
string semantic,
110+
int rowCount,
111+
int columnCount,
112+
int elementCount,
113+
EffectParameterClass parameterClass,
114+
EffectParameterType parameterType,
115+
IntPtr mojoType,
116+
EffectAnnotationCollection annotations,
117+
IntPtr data,
118+
uint dataSizeBytes
119+
) {
120+
if (data == IntPtr.Zero)
121+
{
122+
throw new ArgumentNullException("data");
123+
}
124+
125+
Name = name;
126+
Semantic = semantic ?? string.Empty;
127+
RowCount = rowCount;
128+
ColumnCount = columnCount;
129+
this.elementCount = elementCount;
130+
ParameterClass = parameterClass;
131+
ParameterType = parameterType;
132+
this.mojoType = mojoType;
133+
Annotations = annotations;
134+
values = data;
135+
valuesSizeBytes = dataSizeBytes;
136+
}
137+
89138
internal EffectParameter(
90139
string name,
91140
string semantic,
@@ -108,10 +157,31 @@ uint dataSizeBytes
108157
Semantic = semantic ?? string.Empty;
109158
RowCount = rowCount;
110159
ColumnCount = columnCount;
160+
this.elementCount = elementCount;
161+
ParameterClass = parameterClass;
162+
ParameterType = parameterType;
163+
members = structureMembers;
164+
Annotations = annotations;
165+
values = data;
166+
valuesSizeBytes = dataSizeBytes;
167+
}
168+
169+
#endregion
170+
171+
#region Allocation Optimizations
172+
173+
internal void BuildMemberList()
174+
{
175+
members = Effect.INTERNAL_readEffectParameterStructureMembers(this, mojoType);
176+
}
177+
178+
internal void BuildElementList()
179+
{
111180
if (elementCount > 0)
112181
{
113182
int curOffset = 0;
114183
List<EffectParameter> elements = new List<EffectParameter>(elementCount);
184+
EffectParameterCollection structureMembers = StructureMembers;
115185
for (int i = 0; i < elementCount; i += 1)
116186
{
117187
EffectParameterCollection elementMembers = null;
@@ -138,9 +208,9 @@ uint dataSizeBytes
138208
memElems,
139209
structureMembers[j].ParameterClass,
140210
structureMembers[j].ParameterType,
141-
null, // FIXME: Nested structs! -flibit
211+
IntPtr.Zero, // FIXME: Nested structs! -flibit
142212
structureMembers[j].Annotations,
143-
new IntPtr(data.ToInt64() + curOffset),
213+
new IntPtr(values.ToInt64() + curOffset),
144214
(uint) memSize * 4
145215
));
146216
curOffset += memSize * 4;
@@ -151,28 +221,22 @@ uint dataSizeBytes
151221
elements.Add(new EffectParameter(
152222
null,
153223
null,
154-
rowCount,
155-
columnCount,
224+
RowCount,
225+
ColumnCount,
156226
0,
157227
ParameterClass,
158-
parameterType,
228+
ParameterType,
159229
elementMembers,
160230
null,
161231
new IntPtr(
162-
data.ToInt64() + (i * rowCount * 16)
232+
values.ToInt64() + (i * RowCount * 16)
163233
),
164234
// FIXME: Not obvious to me how to compute this -kg
165235
0
166236
));
167237
}
168-
Elements = new EffectParameterCollection(elements);
238+
this.elements = new EffectParameterCollection(elements);
169239
}
170-
ParameterClass = parameterClass;
171-
ParameterType = parameterType;
172-
StructureMembers = structureMembers;
173-
Annotations = annotations;
174-
values = data;
175-
valuesSizeBytes = dataSizeBytes;
176240
}
177241

178242
#endregion

0 commit comments

Comments
 (0)