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.

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

Already on GitHub? Sign in to your account

feat: improve serialization for errors and bigints #17282

Merged
merged 6 commits into from Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
3 changes: 3 additions & 0 deletions .eslintrc.js
Expand Up @@ -109,6 +109,9 @@ module.exports = {
env: {
"jest/globals": true
},
parserOptions: {
ecmaVersion: 2020
},
TheLarkInn marked this conversation as resolved.
Show resolved Hide resolved
globals: {
nsObj: false,
jasmine: false
Expand Down
30 changes: 30 additions & 0 deletions lib/serialization/BinaryMiddleware.js
Expand Up @@ -84,6 +84,7 @@ const I8_HEADER = 0x60;
const I32_HEADER = 0x40;
const F64_HEADER = 0x20;
const SHORT_STRING_HEADER = 0x80;
const BIGINT_HEADER = 0x32;

/** Uplift high-order bits */
const NUMBERS_HEADER_MASK = 0xe0;
Expand Down Expand Up @@ -331,6 +332,16 @@ class BinaryMiddleware extends SerializerMiddleware {
}
break;
}
case "bigint": {
const value = thing.toString();
const len = Buffer.byteLength(value);
allocate(len + HEADER_SIZE + I32_SIZE);
writeU8(BIGINT_HEADER);
writeU32(len);
currentBuffer.write(value, currentPosition);
currentPosition += len;
break;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Why it is storing as string:

  • we can't use writeBigInt64LE, because BigInt can be longer than 64bits
  • storing by bytes (uint8 or other) will require multiple/shift them in deserialization
  • we can use thing.toString(16) to reduce size, but I doesn't bring a lot of wins, 4294967296n.toString(16) -> 100000000, also it will require to store sing (-/+) and will multiple on -1

so storing them as string, I think, is the best solution

Copy link
Member Author

Choose a reason for hiding this comment

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

Improved, now we bigints as int8 (when n <= BigInt(127) && n >= BigInt(-128)) and as int32 (when n <= BigInt(2147483647) && n >= BigInt(-2147483648)), other bigints we store as a string with ascii characters, so you can store any bigint

case "number": {
const type = identifyNumber(thing);
if (type === 0 && thing >= 0 && thing <= 10) {
Expand Down Expand Up @@ -847,6 +858,25 @@ class BinaryMiddleware extends SerializerMiddleware {
result.push(read(1).readInt8(0));
}
};
case BIGINT_HEADER: {
return () => {
const len = readU32();
if (isInCurrentBuffer(len) && currentPosition + len < 0x7fffffff) {
const value = currentBuffer.toString(
undefined,
currentPosition,
currentPosition + len
);

result.push(BigInt(value));
currentPosition += len;
checkOverflow();
} else {
const value = read(len).toString();
result.push(BigInt(value));
}
};
}
default:
if (header <= 10) {
return () => result.push(header);
Expand Down
3 changes: 3 additions & 0 deletions lib/serialization/ErrorObjectSerializer.js
Expand Up @@ -21,6 +21,7 @@ class ErrorObjectSerializer {
serialize(obj, context) {
context.write(obj.message);
context.write(obj.stack);
context.write(/** @type {Error & { cause: "unknown" }} */ (obj).cause);
}
/**
* @param {ObjectDeserializerContext} context context
Expand All @@ -31,6 +32,8 @@ class ErrorObjectSerializer {

err.message = context.read();
err.stack = context.read();
/** @type {Error & { cause: "unknown" }} */
(err).cause = context.read();

return err;
}
Expand Down
5 changes: 5 additions & 0 deletions lib/serialization/ObjectMiddleware.js
Expand Up @@ -397,6 +397,9 @@ class ObjectMiddleware extends SerializerMiddleware {
", "
)} }`;
}
if (typeof item === "bigint") {
return `BigInt ${item}n`;
}
try {
return `${item}`;
} catch (e) {
Expand Down Expand Up @@ -686,6 +689,8 @@ class ObjectMiddleware extends SerializerMiddleware {
const end1 = read();

if (end1 !== ESCAPE) {
console.log("END" + end1);

throw new Error("Expected end of object");
}

Expand Down
2 changes: 1 addition & 1 deletion lib/serialization/types.js
Expand Up @@ -6,7 +6,7 @@

/** @typedef {undefined|null|number|string|boolean|Buffer|Object|(() => ComplexSerializableType[] | Promise<ComplexSerializableType[]>)} ComplexSerializableType */

/** @typedef {undefined|null|number|string|boolean|Buffer|(() => PrimitiveSerializableType[] | Promise<PrimitiveSerializableType[]>)} PrimitiveSerializableType */
/** @typedef {undefined|null|number|bigint|string|boolean|Buffer|(() => PrimitiveSerializableType[] | Promise<PrimitiveSerializableType[]>)} PrimitiveSerializableType */

/** @typedef {Buffer|(() => BufferSerializableType[] | Promise<BufferSerializableType[]>)} BufferSerializableType */

Expand Down
75 changes: 75 additions & 0 deletions test/Compiler-filesystem-caching.test.js
Expand Up @@ -41,6 +41,81 @@ describe("Compiler (filesystem caching)", () => {
]
};

const isBigIntSupported = typeof BigInt !== "undefined";

options.plugins = [
{
apply(compiler) {
const name = "TestCachePlugin";

compiler.hooks.thisCompilation.tap(name, compilation => {
compilation.hooks.processAssets.tapPromise(
{
name,
stage:
compiler.webpack.Compilation
.PROCESS_ASSETS_STAGE_OPTIMIZE_SIZE
},
async () => {
const cache = compilation.getCache(name);
const ident = "test.ext";
const cacheItem = cache.getItemCache(ident, null);

const result = await cacheItem.getPromise(ident);

if (result) {
expect(result.number).toEqual(42);
expect(result.number1).toEqual(3.14);
expect(result.number2).toEqual(6.2);
expect(result.string).toEqual("string");
expect(result.error.cause.message).toEqual("cause");
expect(result.error1.cause.string).toBe("string");
expect(result.error1.cause.number).toBe(42);

if (isBigIntSupported) {
expect(result.bigint).toEqual(BigInt(123));
expect(result.bigint1).toEqual(
99999999999999999999999999999999999999999999999999991n
);
expect(result.obj.foo).toBe(BigInt(-10));
expect(Array.from(result.set)).toEqual([
BigInt(1),
BigInt(2)
]);
}

return;
}

const storeValue = {};

storeValue.number = 42;
storeValue.string = "string";
storeValue.error = new Error("error", {
cause: new Error("cause")
});
storeValue.error1 = new Error("error", {
cause: { string: "string", number: 42 }
});
storeValue.number1 = 3.14;
storeValue.number2 = 6.2;

if (isBigIntSupported) {
storeValue.bigint = BigInt(123);
storeValue.bigint1 =
99999999999999999999999999999999999999999999999999991n;
storeValue.obj = { foo: BigInt(-10) };
storeValue.set = new Set([BigInt(1), BigInt(2)]);
}

await cacheItem.storePromise(storeValue);
}
);
});
}
}
];

function runCompiler(onSuccess, onError) {
const c = webpack(options);
c.hooks.compilation.tap(
Expand Down