-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add RFC for buffer varint encoding #12
Open
bmcq-0
wants to merge
7
commits into
luau-lang:master
Choose a base branch
from
bmcq-0:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
414bc87
Create function-buffer-varints.md
bmcq-0 f108ef4
Update function-buffer-varints.md
bmcq-0 07aa7e0
Update function-buffer-varints.md
bmcq-0 e1644b6
Update function-buffer-varints.md
bmcq-0 f2d485d
Update function-buffer-varints.md
bmcq-0 86c4f09
Update function-buffer-varints.md
bmcq-0 fd814ce
Update function-buffer-varints.md
bmcq-0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
# Varint Functions for Buffers | ||
|
||
## Summary | ||
|
||
This RFC proposes the addition of new functions to the buffer API: | ||
|
||
- `buffer.readleb128` | ||
- `buffer.readleb128` | ||
- `buffer.readuleb128` | ||
- `buffer.writeuleb128` | ||
|
||
These functions will write and read (U)LEB-128 variable-width integers (varints) from a buffer. | ||
|
||
## Motivation | ||
|
||
One of the top use cases for buffers is compressing and serializing data as small as possible. Numbers consume 8 bytes in their raw form but can be serialized in smaller sizes using buffers to reduce size. However, writing code to serialize as the smallest size can be tedious and confusing for less-experienced programmers. | ||
|
||
The following are implementations of ULEB-128 reading/writing in Luau: | ||
|
||
```lua | ||
local function writeuleb128(stream, value) | ||
while value >= 0x80 do | ||
buffer.writeu8(bit32.bor(bit32.band(value, 0x7f), 0x80)) | ||
value = bit32.rshift(value, 7) | ||
end | ||
buffer.writeu8(value) | ||
end | ||
``` | ||
|
||
```lua | ||
local function readuleb128(stream) | ||
local result = 0 | ||
local bits = 0 | ||
while true do | ||
local byte = buffer.readu8(stream) | ||
result = bit32.bor(result, bit32.lshift(bit32.band(byte, 0x7f), bits)) | ||
if byte < 0x80 then | ||
break | ||
end | ||
bits = bits + 7 | ||
end | ||
return result | ||
end | ||
``` | ||
|
||
The functions above are inefficient and difficult to understand compared to a native implementation. Implementations will also be needed for the corresponding signed functions. | ||
vegorov-rbx marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Design | ||
|
||
The `buffer` library will receive 4 new functions: | ||
|
||
``` | ||
buffer.readleb128(b: buffer, offset: number): number | ||
buffer.readuleb128(b: buffer, offset: number): number | ||
|
||
buffer.writeleb128(b: buffer, offset: number, value: number): () | ||
buffer.writeuleb128(b: buffer, offset: number, value: number): () | ||
``` | ||
|
||
Since other numbers in the buffer library have unsigned and signed implementations, it also makes sense to include both options for varints. | ||
|
||
## Drawbacks | ||
|
||
The only drawback known is a marginal increase in built-in complexity. However, the performance benefit from having native implementations of these functions outweighs the negligible change in complexity and is not a serious concern. | ||
|
||
## Alternatives | ||
|
||
Serialization and deserialization for varints can be recreated directly in Luau. However, the algorithm for doing this may be complicated for less-experienced programmers as it involves bitwise operations. Additionally, the algorithm requires repeated buffer reads and calls to bitwise functions to function correctly, which is far less performant than it could be in native code. | ||
|
||
It is also possible to have a function that serializes a number in the smallest amount of bits it can fit into. However, to read it, the amount of bits it was serialized in would also have to be included. This count of how many bytes to read would also have to be stored in the buffer, which adds an extra byte of unnecessary data. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the implementation examples to match what the proposed functions will do with a library implementation.
Right now they don't compile, incorrectly use the
buffer
library functions and don't return the proposed results.It would be nice to also convert the argument value to integer number using
bit32
library.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bmcq-0 You can update your read implementation with this, adjust accordingly.