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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(JavaScript): Support oneof #1348

Merged
merged 8 commits into from
Jan 26, 2024
Merged

Conversation

theweipeng
Copy link
Member

@theweipeng theweipeng commented Jan 19, 2024

1. Fixed a performance bug that caused writeInt32 to slow down

Before:

// reader/index.ts
function writeVarInt32() {
    buffer.byteLength
}

After:

// reader/index.ts
const byteLength = buffer.byteLength;
function writeVarInt32() {
    byteLength
}

The byteLength property in a Buffer is slow to access. It appears that when we access byteLength, the V8 engine processes it using a hash lookup. so we store it in closure.

2. Support Oneof

Sometimes, the data we want to serialize does not have a confirmed type; instead, it could be one of several confirmed types. If we use an object to handle this situation, the size of the resulting binary will be too large, as it will contain much unused information.

usage:

      const oneOfThree = Type.oneof({
          option1: Type.string(),
          option2: Type.object("foo", {
              a: Type.int32()
          }),
          option3: Type.int32(),
      });
      const fury = new Fury({ refTracking: true });
      const { serialize, deserialize } = fury.registerSerializer(oneOfThree);
      const obj = {
          option1: "hello"
      }
      const input = serialize(obj);
      const result = deserialize(
          input
      );
      expect(result).toEqual(obj.option1)

@bytemain
Copy link
Contributor

I think here has a wrong behavior, the deserialize result should has a kind property also, which indicates that the kind of the raw obj

@theweipeng
Copy link
Member Author

I think here has a wrong behavior, the deserialize result should has a kind property also, which indicates that the kind of the raw obj

The protocol natively supports polymorphism, making a 'kind' field to describe the object type unnecessary.

@bytemain
Copy link
Contributor

ok, It still can meet my needs.

javascript/packages/fury/lib/gen/any.ts Outdated Show resolved Hide resolved
javascript/packages/fury/lib/gen/oneof.ts Show resolved Hide resolved
Copy link
Collaborator

@chaokunyang chaokunyang left a comment

Choose a reason for hiding this comment

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

LGTM

@theweipeng theweipeng merged commit 2f2e60f into apache:main Jan 26, 2024
22 checks passed
theweipeng added a commit to theweipeng/fury that referenced this pull request Feb 14, 2024
### 1. Fixed a performance bug that caused writeInt32 to slow down
Before:
```JavaScript
// reader/index.ts
function writeVarInt32() {
    buffer.byteLength
}
```
After:
```JavaScript
// reader/index.ts
const byteLength = buffer.byteLength;
function writeVarInt32() {
    byteLength
}
```
The byteLength property in a Buffer is slow to access. It appears that
when we access byteLength, the V8 engine processes it using a hash
lookup. so we store it in closure.

### 2.  Support Oneof
Sometimes, the data we want to serialize does not have a confirmed type;
instead, it could be one of several confirmed types. If we use an object
to handle this situation, the size of the resulting binary will be too
large, as it will contain much unused information.

usage:
```JavaScript

      const oneOfThree = Type.oneof({
          option1: Type.string(),
          option2: Type.object("foo", {
              a: Type.int32()
          }),
          option3: Type.int32(),
      });
      const fury = new Fury({ refTracking: true });
      const { serialize, deserialize } = fury.registerSerializer(oneOfThree);
      const obj = {
          option1: "hello"
      }
      const input = serialize(obj);
      const result = deserialize(
          input
      );
      expect(result).toEqual(obj.option1)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants