Skip to content

Commit 9e709de

Browse files
Copilotdadhi
andauthored
refactor: fill ExpressionNode padding with _childIdx/_extraIdx explicit short fields
Replaces the 4-byte alignment padding between NextIdx and _data with two explicit short fields (_childIdx and _extraIdx). Pack=1 is not viable here because the CLR requires managed reference fields at pointer-aligned addresses in arrays. Instead the padding is repurposed as dedicated child/extra index fields: Before: Type(8)+Obj(8)+_typeFlags(2)+NextIdx(2)+[4-byte pad]+_data(8) = 32 bytes After: Type(8)+Obj(8)+_typeFlags(2)+NextIdx(2)+_childIdx(2)+_extraIdx(2)+_data(8) = 32 bytes Benefits: - Zero wasted bytes — same 32-byte size but all bytes are meaningful - _data is now exclusively for inline 8-byte constant storage (cleaner semantics) - ChildIdx/ExtraIdx properties read directly from short fields — no bit shifts - StructurallyEqual updated to compare _childIdx and _extraIdx individually Agent-Logs-Url: https://github.com/dadhi/FastExpressionCompiler/sessions/ec99b322-eedc-4427-8bc8-fd68e2c14865 Co-authored-by: dadhi <39516+dadhi@users.noreply.github.com>
1 parent d83f9b0 commit 9e709de

1 file changed

Lines changed: 16 additions & 11 deletions

File tree

src/FastExpressionCompiler/FlatExpression.cs

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ public static Idx Of(int oneBasedIndex)
7979
}
8080

8181
/// <summary>
82-
/// Compact 32-byte node. Two reference fields first, then two shorts, then the packed 64-bit data word.
83-
/// Layout: Type(8) + Obj(8) + _typeFlags(2) + NextIdx(2) + pad(4) + _data(8) = 32 bytes.
82+
/// Compact 32-byte node. Two reference fields first, then four shorts, then the 64-bit constant word.
83+
/// Layout (no padding waste): Type(8) + Obj(8) + _typeFlags(2) + NextIdx(2) + _childIdx(2) + _extraIdx(2) + _data(8) = 32 bytes.
8484
/// <list type="table">
8585
/// <item><term>Constant inline</term>
8686
/// <description>IsInplaceConst = true; <see cref="Data"/> holds raw bits (up to 8 bytes: bool/int/long/float/double/DateTime/…).</description></item>
@@ -100,7 +100,7 @@ public static Idx Of(int oneBasedIndex)
100100
/// <para>vs LightExpression heap objects (16-byte GC header + fields): Constant/Parameter ~40 bytes | Binary/Unary ~48–56 bytes.</para>
101101
/// </summary>
102102
[StructLayout(LayoutKind.Sequential)]
103-
public struct ExpressionNode // 32 bytes: Type(8)+Obj(8)+_typeFlags(2)+NextIdx(2)+pad(4)+_data(8)
103+
public struct ExpressionNode // 32 bytes: Type(8)+Obj(8)+_typeFlags(2)+NextIdx(2)+_childIdx(2)+_extraIdx(2)+_data(8)
104104
{
105105
/// <summary>Result type of this node.</summary>
106106
public Type Type;
@@ -110,21 +110,20 @@ public struct ExpressionNode // 32 bytes: Type(8)+Obj(8)+_typeFlags(2)+NextIdx(
110110
internal short _typeFlags;
111111
/// <summary>Raw next-sibling index (0 = nil).</summary>
112112
public short NextIdx;
113-
// Packed 16-bit indexes (or full 8-byte constant bits when IsInplaceConst):
114-
// bits 0–15: ChildIdx (first child / 1-based closure slot for Constant)
115-
// bits 16–31: ChildCount (reserved, future use)
116-
// bits 32–47: ExtraIdx (second child; nil for Unary and non-control nodes)
117-
// bits 48–63: ExtraCount (reserved, future use)
113+
// Explicit shorts fill the 4 bytes that would otherwise be padding before the long.
114+
internal short _childIdx; // first child, or 1-based closure slot for Constant
115+
internal short _extraIdx; // second child; nil for Unary and non-control nodes
116+
// Raw 8-byte constant bits when IsInplaceConst is true; zero for all other node types.
118117
internal long _data;
119118

120119
/// <summary>Expression kind.</summary>
121120
public ExpressionType NodeType => (ExpressionType)(_typeFlags & 0x7FFF);
122121
/// <summary>True when this Constant node's value is stored inline in <see cref="Data"/>.</summary>
123122
public bool IsInplaceConst => (_typeFlags & 0x8000) != 0;
124123
/// <summary>First child, or 1-based closure slot for non-inline Constant nodes.</summary>
125-
public Idx ChildIdx => Idx.Of((int)(ushort)(_data & 0xFFFF));
124+
public Idx ChildIdx => Idx.Of((int)(ushort)_childIdx);
126125
/// <summary>Second child (nil for unary nodes and for New/Call/Invoke args — only used for control nodes).</summary>
127-
public Idx ExtraIdx => Idx.Of((int)(ushort)((_data >> 32) & 0xFFFF));
126+
public Idx ExtraIdx => Idx.Of((int)(ushort)_extraIdx);
128127
/// <summary>Raw 8-byte constant bits when <see cref="IsInplaceConst"/> is true.</summary>
129128
public long Data => _data;
130129
}
@@ -168,7 +167,9 @@ private Idx AddNode(
168167
n._typeFlags = (short)nodeType;
169168
n.Type = type;
170169
n.Obj = obj;
171-
n._data = ((long)(ushort)extraIdx.It << 32) | (ushort)childIdx.It;
170+
n._childIdx = childIdx.It;
171+
n._extraIdx = extraIdx.It;
172+
n._data = 0;
172173
n.NextIdx = 0;
173174
return Idx.Of(Nodes.Count); // Count already incremented by AddDefaultAndGetRef
174175
}
@@ -267,6 +268,8 @@ public Idx Constant(object value, bool putIntoClosure = false)
267268
n._typeFlags = unchecked((short)((int)ExpressionType.Constant | 0x8000));
268269
n.Type = type;
269270
n.Obj = null;
271+
n._childIdx = 0;
272+
n._extraIdx = 0;
270273
n._data = ToInt64Bits(value, type);
271274
n.NextIdx = 0;
272275
return Idx.Of(Nodes.Count);
@@ -793,6 +796,8 @@ public static bool StructurallyEqual(ref ExpressionTree a, ref ExpressionTree b)
793796
if (na.Type != nb.Type) return false;
794797
if (!ObjEqual(na.Obj, nb.Obj)) return false;
795798
if (na.NextIdx != nb.NextIdx) return false;
799+
if (na._childIdx != nb._childIdx) return false;
800+
if (na._extraIdx != nb._extraIdx) return false;
796801
if (na._data != nb._data) return false;
797802
}
798803
for (var i = 0; i < a.ClosureConstants.Count; i++)

0 commit comments

Comments
 (0)