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

Jump to bottom

feat: fast ArrayBuffer encoding #566

Merged
merged 10 commits into from May 11, 2022
Merged

feat: fast ArrayBuffer encoding #566

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ffc05c1
Special case ArrayBuffer handling in encoding
ninegua Apr 22, 2022
3f1aaae
Fix
ninegua Apr 22, 2022
50c063f
Fix
ninegua Apr 22, 2022
ddefc02
Fix bits checking
ninegua Apr 22, 2022
38c03dc
Faster encoding for ArrayBuffer other than Uint8Array
ninegua Apr 22, 2022
a1e97d6
Fast decoding path too
ninegua Apr 22, 2022
45056bf
Suppor BigUint64Array and BigInt64Array
ninegua Apr 22, 2022
2717f3e
Update packages/candid/src/idl.test.ts
ninegua Apr 22, 2022
2375a33
Add doc string on array encoding/decoding
ninegua Apr 22, 2022
f7634f2
Comment on _blobOptimization
ninegua Apr 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions packages/candid/src/idl.test.ts
View file
Open in desktop
Expand Up @@ -152,6 +152,20 @@ test('IDL encoding (tuple)', () => {
);
});

test('IDL encoding (arraybuffer)', () => {
// ArrayBuffer, encode only.
ninegua marked this conversation as resolved.
Show resolved Hide resolved
testEncode(
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't change the decoding behavior because it will be a breaking change.

But I will suggest decoding should be changed too. What do people think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Commit a1e97d6 adds a fast decoding path too. This changes the return type from Array to ArrayBuffers, and will likely break user code when they upgrade.

IDL.Vec(IDL.Int8),
new Int8Array([0, 1, 2, 3]),
'4449444c016d7701000400010203',
'Array of Ints',
);
IDL.encode([IDL.Vec(IDL.Nat8)], [new Uint8Array()]);
IDL.encode([IDL.Vec(IDL.Nat8)], [new Uint8Array(100).fill(42)]);
IDL.encode([IDL.Vec(IDL.Nat16)], [new Uint16Array(200).fill(42)]);
expect(() => IDL.encode([IDL.Vec(IDL.Int8)], [new Uint16Array(10).fill(420)])).toThrow(/Invalid vec int8 argument/);
});

test('IDL encoding (array)', () => {
// Array
test_(
Expand Down
4 changes: 3 additions & 1 deletion packages/candid/src/idl.ts
View file
Open in desktop
Expand Up @@ -742,7 +742,9 @@ export class VecClass<T> extends ConstructType<T[]> {
}

public covariant(x: any): x is T[] {
return Array.isArray(x) && x.every(v => this._type.covariant(v));
// Special case for ArrayBuffer
return (ArrayBuffer.isView(x) && (x.byteLength == 0 || this._type.covariant(x[0])))
|| (Array.isArray(x) && x.every(v => this._type.covariant(v)));
}

public encodeValue(x: T[]) {
Expand Down