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

Fix TypeScript buffer bug #225

Closed
wants to merge 3 commits into from
Closed

Fix TypeScript buffer bug #225

wants to merge 3 commits into from

Conversation

andrewmd5
Copy link
Contributor

As outlined in #209, the TypeScript runtime utilizes a shared buffer for encoding which can lead to subtle bugs when decoding multiple types. For example:

    // should be the data of 'CreateUser'
    const createUserBuffer = CreateUser.encode({ email: "hi@andrew.im" });
    // encode another type
    CreateOk.encode({ now: 1 })
    // fails to decode as the data of 'createUserBuffer' is now the data of 'CreateOk.encode'
    const user = CreateUser.decode(createUserBuffer);

This change ensures we create and return a new buffer from 'toArray()'; yes, it allocates additional data, but it means users don't need to think about what happens when encoding multiple types when they might still need the data buffer.

As outlined in #209, the TypeScript runtime utilizes a shared buffer for encoding which can lead to subtle bugs when decoding multiple types. For example:
```typescript
    // should be the data of 'CreateUser'
    const createUserBuffer = CreateUser.encode({ email: "hi@andrew.im" });
    // encode another type
    CreateOk.encode({ now: 1 })
    // fails to decode as the data of 'createUserBuffer' is now the data of 'CreateOk.encode'
    const user = CreateUser.decode(createUserBuffer);
```

This change ensures we create and return a new buffer from 'toArray()'; yes, it allocates additional data, but it means users don't need to think about what happens when encoding multiple types when they might still need the data buffer.
@andrewmd5 andrewmd5 added the typescript Issues regarding Typescript label Apr 7, 2023
@andrewmd5
Copy link
Contributor Author

andrewmd5 commented Apr 7, 2023

Holding off on merging this as it introduces a major performance hit

Bebop (subarray)   encode Song x 7,225,130 ops/sec ±0.08% (101 runs sampled)
Bebop (subarray)   decode Song x 2,861,336 ops/sec ±0.11% (100 runs sampled)
Bebop (slice)   encode Song x 2,259,026 ops/sec ±2.66% (80 runs sampled)
Bebop (slice)  decode Song x 2,823,515 ops/sec ±0.28% (94 runs sampled)

@andrewmd5
Copy link
Contributor Author

opting for a different approach

@andrewmd5 andrewmd5 closed this Jun 27, 2023
@andrewmd5 andrewmd5 deleted the ts-buffer-bug branch January 22, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Issues regarding Typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant