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

Remove uses of LazyMessageReader #29

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Conversation

jtbandes
Copy link
Member

@jtbandes jtbandes commented Oct 4, 2023

Public-Facing Changes

Received messages are now fully deserialized as JS objects rather than using @foxglove/rosmsg-serialization's LazyMessageReader.

Description

Resolves FG-5047
Ultimately we did not find that lazy reading provided a performance benefit that justifies the downstream complexity of inconsistencies between data sources. The Studio app will likely need a deeper architectural change to support more efficient message processing across worker threads, rather than just swapping out objects for lazy objects.

@jtbandes jtbandes merged commit b7dc23d into main Oct 5, 2023
2 checks passed
@jtbandes jtbandes deleted the jacob/remove-lazymessage branch October 5, 2023 17:14
pezy pushed a commit to pezy/studio that referenced this pull request Oct 25, 2023
**User-Facing Changes**
None

**Description**
Depends on foxglove/ros1#29
Resolves FG-5044

The ROS 1 native and Rosbridge sources were the only places using "lazy"
message reading, which required a `toJSON` dance before sending messages
to workers. There are several places we can remove this if we stop using
lazy message reading.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants