-
Notifications
You must be signed in to change notification settings - Fork 81
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
Improve throughput performance #2 (refcount focus) #737
Merged
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
PR missing one of the required labels: {'new feature', 'breaking-change', 'bug', 'documentation', 'enhancement', 'dependencies', 'internal'} |
jean-roland
force-pushed
the
ft_rc_perf
branch
from
October 12, 2024 08:50
bb30352
to
4e88a66
Compare
DenisBiryukov91
approved these changes
Oct 14, 2024
sashacmc
reviewed
Oct 14, 2024
sashacmc
approved these changes
Oct 14, 2024
jean-roland
added a commit
to jean-roland/zenoh-pico
that referenced
this pull request
Nov 13, 2024
…pse-zenoh#737) * feat: more static inline * feat: skip null string copy in encoding * feat: add no-weak refcount * feat: streamline vec_make * feat: add publisher check session config token * fix: simple rc memory leak * feat: switch arc_slice to simple rc * feat: add valid flag to timestamp * feat: add timestamp_move function * feat: switch sample_create to pass by reference * fix: missing token on publisher_delete * feat: update trigger subscription calls * fix: dummy sample_create prototype * feat: remove null timestamp value in trigger subs * feat: optimize timestamps cost * feat: check string before encoding move * feat: alias instead of copy payload on decode * doc: explicit read task errors * doc: rework transport/codec logs * feat: keep going on multicast message processing error * fix: revert payload aliasing * fix: set timestamp valid only if decode successful * fix: review comment
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 PR continues on the work of #730. Profiling helped identify multiple causes of performance issue, including our refcount implementation with weak pointers.
More static inline rework:
Similar work to what was done previously.
Lazier & faster clear / initialization:
Some types were taking time initializing or clearing things that weren't necessary. They got simplified.
Chief of this was timestamps. A validity boolean flag was added to the struct to simplify checks & clears of the type.
"Simple rc" - name subject to change:
Added back a rc implementation without weak, used only for arc_slices. Closes Add back a refcount implementation without weak feature #520
Add
Z_FEATURE_PUBLISHER_SESSION_CHECK
config tokenOn by default, this can be turned off to avoid upgrading the publisher's session weak in apps where session is not dropped before it stops publishing.
Reworked logs from the transport/codec layer.
Multicast read task more tolerant:
As pico multicast is on a
best-effort
transport only, a missed fragment shouldn't close the connection (it was causing tests to halt).