Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(store): refactor tightcoder, optimize record encoder #1183

Closed
wants to merge 14 commits into from
Closed

refactor(store): refactor tightcoder, optimize record encoder #1183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1f9093e
update tightcoder renderers
dk1a Jul 19, 2023
80bad46
add abiEncodeTightlyPacked, partially refactor tightcoder
dk1a Jul 21, 2023
ea0384b
test(store): update GasTest
dk1a Jul 24, 2023
19d81c3
try to avoid unnecessary memory copy when encoding record
dk1a Jul 24, 2023
1831958
improve memory copy, some cleanup
dk1a Jul 25, 2023
6a37b4e
improve abiType loops in renderers
dk1a Jul 25, 2023
d7f808a
clean up tightcoder tests
dk1a Jul 25, 2023
4e2389e
unwrap type in renderEncodeRecord
dk1a Jul 25, 2023
905823d
renderDynamicLength
dk1a Jul 25, 2023
08c2e62
build
dk1a Jul 25, 2023
b840cd5
cleanup
dk1a Jul 25, 2023
7c8e85f
build(store): run generate-tightcoder in build script
dk1a Jul 25, 2023
70cdeaf
cleanup
dk1a Jul 25, 2023
819a36b
Create forty-bikes-push.md
dk1a Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/forty-bikes-push.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@latticexyz/cli": patch
"@latticexyz/common": patch
"@latticexyz/store": patch
---

Refactor tightcoder to use typescript functions instead of ejs
Optimize `encode` function in generated table libraries using a new `renderEncodeRecord` codegen function
Add `isLeftAligned` and `shiftLeftBits` common codegen helpers
27 changes: 24 additions & 3 deletions e2e/packages/contracts/src/codegen/tables/NumberList.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,30 @@ library NumberList {
function encode(uint32[] memory value) internal pure returns (bytes memory) {
uint40[] memory _counters = new uint40[](1);
_counters[0] = uint40(value.length * 4);
PackedCounter _encodedLengths = PackedCounterLib.pack(_counters);

return abi.encodePacked(_encodedLengths.unwrap(), EncodeArray.encode((value)));
bytes32 _encodedLengths = PackedCounterLib.pack(_counters).unwrap();

uint256 _resultLength;
unchecked {
_resultLength = 32 + value.length * 4;
}

bytes memory _result;
uint256 _resultPointer;

/// @solidity memory-safe-assembly
assembly {
// allocate memory
_result := mload(0x40)
_resultPointer := add(_result, 0x20)
mstore(0x40, add(_resultPointer, and(add(_resultLength, 31), not(31))))
mstore(_result, _resultLength)

mstore(add(_resultPointer, 0), shl(0, _encodedLengths))

_resultPointer := add(_resultPointer, 32)
}
EncodeArray.encodeToLocation((value), _resultPointer);
return _result;
Comment on lines +175 to +198
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one general thought: having complex / low level logic like this in code that is produced by codegeneration (even if it's literally just a template string) is gonna be a red flag for folks during the audit. Is there any way you can think of how we could encapsulate this in a general function outside of codegen that can be audited separately, so that codegen code looks as simple as possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this is an interesting/unusual situation. Essentially I'm doing what solidity does with its IR codegen, but it's scuffed because I don't have access to all the solidity's yul utils. This part of MUD is more a language compiler / addition to solidity, than just a contract.
The encapsulated version would be what we had before this PR. This is not strictly better than before and I have considered this as a reason to reject this PR if you don't think we should do compiler-like yul codegen in MUD

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLDR no, this is a critical part of the PR. You can just reject it, I'll make a much simpler PR refactoring ejs to typescript and some other minor things

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say if this is causing dramatic improvements in gas we should consider it, if the improvements are modest we should optimize for simplicity in the generated code

}

/** Encode keys as a bytes32 array using this table's schema */
Expand Down
27 changes: 24 additions & 3 deletions examples/minimal/packages/contracts/src/codegen/tables/MessageTable.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,30 @@ library MessageTable {
function encode(string memory value) internal pure returns (bytes memory) {
uint40[] memory _counters = new uint40[](1);
_counters[0] = uint40(bytes(value).length);
PackedCounter _encodedLengths = PackedCounterLib.pack(_counters);

return abi.encodePacked(_encodedLengths.unwrap(), bytes((value)));
bytes32 _encodedLengths = PackedCounterLib.pack(_counters).unwrap();

uint256 _resultLength;
unchecked {
_resultLength = 32 + bytes(value).length;
}

bytes memory _result;
uint256 _resultPointer;

/// @solidity memory-safe-assembly
assembly {
// allocate memory
_result := mload(0x40)
_resultPointer := add(_result, 0x20)
mstore(0x40, add(_resultPointer, and(add(_resultLength, 31), not(31))))
mstore(_result, _resultLength)

mstore(add(_resultPointer, 0), shl(0, _encodedLengths))

_resultPointer := add(_resultPointer, 32)
}
Memory.copy(Memory.dataPointer(bytes((value))), _resultPointer, bytes(value).length);
return _result;
}

/** Encode keys as a bytes32 array using this table's schema */
Expand Down
62 changes: 51 additions & 11 deletions packages/cli/contracts/src/codegen/tables/Dynamics1.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,17 +781,57 @@ library Dynamics1 {
_counters[2] = uint40(staticU128.length * 16);
_counters[3] = uint40(staticAddrs.length * 20);
_counters[4] = uint40(staticBools.length * 1);
PackedCounter _encodedLengths = PackedCounterLib.pack(_counters);

return
abi.encodePacked(
_encodedLengths.unwrap(),
EncodeArray.encode(fromStaticArray_bytes32_1(staticB32)),
EncodeArray.encode(fromStaticArray_int32_2(staticI32)),
EncodeArray.encode(fromStaticArray_uint128_3(staticU128)),
EncodeArray.encode(fromStaticArray_address_4(staticAddrs)),
EncodeArray.encode(fromStaticArray_bool_5(staticBools))
);
bytes32 _encodedLengths = PackedCounterLib.pack(_counters).unwrap();

uint256 _resultLength;
unchecked {
_resultLength =
32 +
staticB32.length *
32 +
staticI32.length *
4 +
staticU128.length *
16 +
staticAddrs.length *
20 +
staticBools.length *
1;
}

bytes memory _result;
uint256 _resultPointer;

/// @solidity memory-safe-assembly
assembly {
// allocate memory
_result := mload(0x40)
_resultPointer := add(_result, 0x20)
mstore(0x40, add(_resultPointer, and(add(_resultLength, 31), not(31))))
mstore(_result, _resultLength)

mstore(add(_resultPointer, 0), shl(0, _encodedLengths))

_resultPointer := add(_resultPointer, 32)
}
EncodeArray.encodeToLocation(fromStaticArray_bytes32_1(staticB32), _resultPointer);
unchecked {
_resultPointer += staticB32.length * 32;
}
EncodeArray.encodeToLocation(fromStaticArray_int32_2(staticI32), _resultPointer);
unchecked {
_resultPointer += staticI32.length * 4;
}
EncodeArray.encodeToLocation(fromStaticArray_uint128_3(staticU128), _resultPointer);
unchecked {
_resultPointer += staticU128.length * 16;
}
EncodeArray.encodeToLocation(fromStaticArray_address_4(staticAddrs), _resultPointer);
unchecked {
_resultPointer += staticAddrs.length * 20;
}
EncodeArray.encodeToLocation(fromStaticArray_bool_5(staticBools), _resultPointer);
return _result;
}

/** Encode keys as a bytes32 array using this table's schema */
Expand Down
33 changes: 31 additions & 2 deletions packages/cli/contracts/src/codegen/tables/Dynamics2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,9 +508,38 @@ library Dynamics2 {
_counters[0] = uint40(u64.length * 8);
_counters[1] = uint40(bytes(str).length);
_counters[2] = uint40(bytes(b).length);
PackedCounter _encodedLengths = PackedCounterLib.pack(_counters);
bytes32 _encodedLengths = PackedCounterLib.pack(_counters).unwrap();

return abi.encodePacked(_encodedLengths.unwrap(), EncodeArray.encode((u64)), bytes((str)), bytes((b)));
uint256 _resultLength;
unchecked {
_resultLength = 32 + u64.length * 8 + bytes(str).length + bytes(b).length;
}

bytes memory _result;
uint256 _resultPointer;

/// @solidity memory-safe-assembly
assembly {
// allocate memory
_result := mload(0x40)
_resultPointer := add(_result, 0x20)
mstore(0x40, add(_resultPointer, and(add(_resultLength, 31), not(31))))
mstore(_result, _resultLength)

mstore(add(_resultPointer, 0), shl(0, _encodedLengths))

_resultPointer := add(_resultPointer, 32)
}
EncodeArray.encodeToLocation((u64), _resultPointer);
unchecked {
_resultPointer += u64.length * 8;
}
Memory.copy(Memory.dataPointer(bytes((str))), _resultPointer, bytes(str).length);
unchecked {
_resultPointer += bytes(str).length;
}
Memory.copy(Memory.dataPointer(bytes((b))), _resultPointer, bytes(b).length);
return _result;
}

/** Encode keys as a bytes32 array using this table's schema */
Expand Down
44 changes: 34 additions & 10 deletions packages/cli/contracts/src/codegen/tables/Singleton.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,16 +488,40 @@ library Singleton {
_counters[0] = uint40(v2.length * 4);
_counters[1] = uint40(v3.length * 4);
_counters[2] = uint40(v4.length * 4);
PackedCounter _encodedLengths = PackedCounterLib.pack(_counters);

return
abi.encodePacked(
v1,
_encodedLengths.unwrap(),
EncodeArray.encode(fromStaticArray_uint32_2(v2)),
EncodeArray.encode(fromStaticArray_uint32_2(v3)),
EncodeArray.encode(fromStaticArray_uint32_1(v4))
);
bytes32 _encodedLengths = PackedCounterLib.pack(_counters).unwrap();

uint256 _resultLength;
unchecked {
_resultLength = 64 + v2.length * 4 + v3.length * 4 + v4.length * 4;
}

bytes memory _result;
uint256 _resultPointer;

/// @solidity memory-safe-assembly
assembly {
// allocate memory
_result := mload(0x40)
_resultPointer := add(_result, 0x20)
mstore(0x40, add(_resultPointer, and(add(_resultLength, 31), not(31))))
mstore(_result, _resultLength)

mstore(add(_resultPointer, 0), shl(0, v1))

mstore(add(_resultPointer, 32), shl(0, _encodedLengths))

_resultPointer := add(_resultPointer, 64)
}
EncodeArray.encodeToLocation(fromStaticArray_uint32_2(v2), _resultPointer);
unchecked {
_resultPointer += v2.length * 4;
}
EncodeArray.encodeToLocation(fromStaticArray_uint32_2(v3), _resultPointer);
unchecked {
_resultPointer += v3.length * 4;
}
EncodeArray.encodeToLocation(fromStaticArray_uint32_1(v4), _resultPointer);
return _result;
}

/** Encode keys as a bytes32 array using this table's schema */
Expand Down
12 changes: 12 additions & 0 deletions packages/common/src/codegen/render-solidity/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,18 @@ export function renderValueTypeToBytes32(name: string, { typeUnwrap, internalTyp
}
}

export function isLeftAligned(field: Pick<RenderType, "internalTypeId">): boolean {
return field.internalTypeId.match(/^bytes\d{1,2}$/) !== null;
}

export function shiftLeftBits(field: Pick<RenderType, "internalTypeId" | "staticByteLength">): number {
if (isLeftAligned(field)) {
return 0;
} else {
return 256 - field.staticByteLength * 8;
}
}

function internalRenderList<T>(
lineTerminator: string,
list: T[],
Expand Down
Loading