feat(v8/scripting): swap from msgpack-lite to msgpackr #3018
+2,445
−44
Conversation
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
Co-authored-by: Linden <[email protected]>
github-actions
bot
added
ScRT: JS
thelindat
reviewed
Dec 19, 2024
Comment on lines
+846
to
+847
| throw new Error('Unknown extension type ' + type) |
There was a problem hiding this comment.
fwiw this will break compatibility for any type that has no extension. For example, unpacking a vector2 from Lua results in
return {
"buffer": {
"type": "Buffer",
"data": number[] // i.e. [11,205,143,63,183,230,7,64]
},
"type": 20
}I will be submitting a "breaking" PR for vectors anyway to actually support them, but might cause some complaints (e.g. where those buffers are manually handled).
There was a problem hiding this comment.
We should probably just handle the conversions here anyways, since sending vectors to/from scrts is currently very hacky and I doubt may people do it currently.
There was a problem hiding this comment.
Created an array containing 100 objects and another with 1000 objects. Performed 1000 iterations.
msgpack-lite
Running benchmark for packing...
Pack - Min: 0.0568 ms, Max: 1.1691 ms, Avg: 0.080365 ms
Running benchmark for unpacking...
Unpack - Min: 0.0482 ms, Max: 1.1444 ms, Avg: 0.061173 ms
Running benchmark for packing...
Pack - Min: 5.2618 ms, Max: 13.8522 ms, Avg: 5.99034 ms
Running benchmark for unpacking...
Unpack - Min: 5.0637 ms, Max: 20.6582 ms, Avg: 5.366889 ms
msgpackr
Running benchmark for packing...
Pack - Min: 0.0185 ms, Max: 4.2132 ms, Avg: 0.03549 ms
Running benchmark for unpacking...
Unpack - Min: 0.035 ms, Max: 1.9161 ms, Avg: 0.059167 ms
Running benchmark for packing...
Pack - Min: 1.8215 ms, Max: 10.3579 ms, Avg: 1.995244 ms
Running benchmark for unpacking...
Unpack - Min: 3.4534 ms, Max: 18.5718 ms, Avg: 3.717805 ms
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Goal of this PR
Swap JS to msgpackr to allow serializing classes & other js builtin objects (i.e.
Map,Set,Error, etc) and also increase performance.How is this PR achieving the goal
Swap out the custom version of msgpack-lite to use msgpackr
This PR applies to the following area(s)
ScRT: JS
Successfully tested on
Game builds: N/A
Platforms: Windows, Linux
Checklist
Fixes issues
Supersedes PR #2931