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

Make wrapped lists in PbList monomorphic #902

Merged
merged 6 commits into from Nov 23, 2023

Conversation

osa1
Copy link
Member

@osa1 osa1 commented Nov 20, 2023

In PbList, the list field becomes monomorphic growable list (from the list base class). This makes add calls monomorphic and inlinable, and avoids double mutability checks (once in PbList.add, again in _wrappedList.add).

Also simplifies immutable PbMap allocations.

PbMap._isReadonly is renamed to _isReadOnly for consistency with PbList._isReadOnly.


cl/585873554

Use the same allocation syntax in all allocation sites to make sure one
implementation class will flow into the fields.

For `PbMap`, this doesn't make any difference, but makes the intention
clear. Map calls are currently already monomorphic.

For `PbList`, the list field becomes monomorphic growable list (from the
list base class). This makes `add` calls monomorphic and inlinable, and
avoids double mutability checks (once in `PbList.add`, again in
`_wrappedList.add`).
protobuf/lib/src/protobuf/pb_list.dart Outdated Show resolved Hide resolved
protobuf/lib/src/protobuf/pb_list.dart Outdated Show resolved Hide resolved
@osa1 osa1 changed the title Make wrapped lists/maps in PbList/PbMap monomorphic Make wrapped lists in PbList monomorphic Nov 22, 2023
@osa1
Copy link
Member Author

osa1 commented Nov 22, 2023

@mkustermann I mostly rewrote this PR, ptal.

osa1 added a commit to osa1/protobuf.dart that referenced this pull request Nov 22, 2023
This improves iteration performance by making `moveNext` and `current`
direct calls in kernel. google#902 does not have the same effect on `moveNext`
and `current` calls, this change is needed even with google#902.

This change was not possible before google#880 as users could override the
list and map types to types that are not `PbList`s or `PbMap`s.
protobuf/lib/src/protobuf/pb_list.dart Outdated Show resolved Hide resolved
protobuf/lib/src/protobuf/pb_map.dart Outdated Show resolved Hide resolved
@osa1 osa1 merged commit 4e0bdff into google:master Nov 23, 2023
17 of 18 checks passed
@osa1 osa1 deleted the osa1/map_list_impl_classes branch November 23, 2023 08:24
osa1 added a commit that referenced this pull request Nov 23, 2023
This improves iteration performance by making `moveNext` and `current` direct
calls in kernel. #902 does not have the same effect on `moveNext` and `current`
calls, this change is needed even with #902.

This change was not possible before #880 as users could override the list and
map types to types that are not `PbList`s or `PbMap`s.

Note: changes in the message field types (the plugin changes in this PR) are
not necessary for the kernel improvements, but it doesn't hurt to generate more
precise types everywhere.
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.

None yet

2 participants