Convert internal asset, dependency, and environment fields to numbers #9663
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
# Conflicts: # packages/core/core/src/public/Asset.js # packages/core/core/src/public/Dependency.js
yamadapc
reviewed
Apr 23, 2024
| @@ -123,7 +124,55 @@ class BaseAsset { | |||
| } | |||
|
|
|||
| get meta(): Meta { | |||
| return this.#asset.value.meta; | |||
| return new Proxy(this.#asset.value.meta, { | |||
There was a problem hiding this comment.
It is possible the use of proxies offsets any improvement this would have as far as runtime is concerned
There was a problem hiding this comment.
The main goal of this PR is to reduce the size of these data structures, in memory and when serialized, and also align them with how a representation could work in Rust. I don't think there should be much difference in performance in terms of time. This came from the ParcelDB branch where I believe Atlassian saw a fairly significant performance improvement though (likely not because of this in particular but it's a good sign). But if you have a better idea on how to implement this backwards compatibility let me know.
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.
This converts some fields on our internal Asset, Dependency, and Environment types from strings/booleans to numbers/bitfields. This is mostly in preparation for the conversion to Rust, where we will want more efficient serialization, but in the interim this should also reduce the size of some objects in the JS implementation too. This was started on the Parcel DB branch but should be useful even when using something like Serde too.
Currently, for backward compatibility, some
asset.metaanddependency.metafields are proxied to internal bitflags transparently to plugins. We may eventually want to expose more official fields for these things but that can be discussed at a later time.