-
Notifications
You must be signed in to change notification settings - Fork 31
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
Early unsubscribe #258
Early unsubscribe #258
Conversation
Codecov Report
@@ Coverage Diff @@
## main #258 +/- ##
==========================================
- Coverage 98.43% 98.20% -0.23%
==========================================
Files 92 93 +1
Lines 1338 1340 +2
==========================================
- Hits 1317 1316 -1
- Misses 21 24 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
BENCHMARK RESULTS (AUTOGENERATED)ci-ubuntu-clangObservable constructionTable
Observable liftTable
Observable subscribeTable
Observable subscribe #2Table
Observer constructionTable
OnNextTable
Subscriber constructionTable
SubscriptionTable
bufferTable
chains creation testTable
combine_latestTable
concatTable
distinct_until_changedTable
firstTable
foundamental sourcesTable
fromTable
immediate schedulerTable
justTable
mapTable
mergeTable
observe_onTable
publish_subject callbacksTable
publish_subject routinesTable
repeatTable
scanTable
skipTable
switch_on_nextTable
takeTable
take_lastTable
trampoline schedulerTable
windowTable
with_latest_fromTable
ci-ubuntu-gccObservable constructionTable
Observable liftTable
Observable subscribeTable
Observable subscribe #2Table
Observer constructionTable
OnNextTable
Subscriber constructionTable
SubscriptionTable
bufferTable
chains creation testTable
combine_latestTable
concatTable
distinct_until_changedTable
firstTable
foundamental sourcesTable
fromTable
immediate schedulerTable
justTable
mapTable
mergeTable
observe_onTable
publish_subject callbacksTable
publish_subject routinesTable
repeatTable
scanTable
skipTable
switch_on_nextTable
takeTable
take_lastTable
trampoline schedulerTable
windowTable
with_latest_fromTable
ci-windowsObservable constructionTable
Observable liftTable
Observable subscribeTable
Observable subscribe #2Table
Observer constructionTable
OnNextTable
Subscriber constructionTable
SubscriptionTable
bufferTable
chains creation testTable
combine_latestTable
concatTable
distinct_until_changedTable
firstTable
foundamental sourcesTable
fromTable
immediate schedulerTable
justTable
mapTable
mergeTable
observe_onTable
publish_subject callbacksTable
publish_subject routinesTable
repeatTable
scanTable
skipTable
switch_on_nextTable
takeTable
take_lastTable
trampoline schedulerTable
windowTable
with_latest_fromTable
|
|
||
state->count_of_on_completed_needed.fetch_add(1, std::memory_order::relaxed); | ||
|
||
auto subscription = subscriber.get_subscription().make_child(); | ||
auto subscription = state->childs_subscriptions.make_child(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the component diagram for subscription from the original behavior.
In the new behavior, you create merge_sub_child_1 --> merge_sub_child_2
and then early-unsubscribe to merge_sub_child_1
(will unsubscribe merge_sub_child_2
eventually). Is it too many layers?
Note I use "scbr" for subscriber and "sub" for subscription.
{ original } { new }
sub_a sub_b sub_a sub_b
\ / \ /
merge_sub merge_sub_2 <-- seen by merge_scbr
| |
| merge_sub_1 <-- used for early unsubscribing
| |
sub sub
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. I plot the subscription tree wrong. It's a brilliant idea and the correct plot should be
{ original } { new }
sub_a sub_b sub_a sub_b
\ / | |
merge_sub \ merge_sub_2 / <-- seen by merge_scbr
| \ | /
| merge_sub_1 <-- used for early unsubscribing
| |
sub sub
Kudos, SonarCloud Quality Gate passed! |
No description provided.