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

refactor: Make event dispatch ordered by receive time. #2392

Merged
merged 3 commits into from
Jan 15, 2024

Conversation

iphydf
Copy link
Member

@iphydf iphydf commented Aug 22, 2023

This change is Reviewable

@iphydf iphydf added this to the v0.2.19 milestone Aug 22, 2023
@iphydf iphydf force-pushed the ordered-events branch 2 times, most recently from 8e372bc to 07f91b7 Compare August 22, 2023 19:39
@Green-Sky
Copy link
Member

Green-Sky commented Aug 22, 2023

will first port my #2378 to this change and then give review done.

@iphydf iphydf force-pushed the ordered-events branch 6 times, most recently from 919e79b to beb0ef9 Compare August 23, 2023 09:51
@iphydf iphydf marked this pull request as draft August 23, 2023 11:08
@iphydf iphydf changed the title refactor: Make event dispatch ordered by receive time. [WIP] refactor: Make event dispatch ordered by receive time. Aug 23, 2023
@iphydf iphydf force-pushed the ordered-events branch 6 times, most recently from 2436d0d to c928ff7 Compare August 31, 2023 00:12
@iphydf iphydf modified the milestones: v0.2.19, v0.2.20 Nov 7, 2023
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 2078 lines in your changes are missing coverage. Please review.

Comparison is base (001d00a) 71.11% compared to head (b4d8826) 66.78%.

Files Patch % Lines
toxcore/tox_event.c 39.14% 384 Missing ⚠️
toxcore/events/group_message.c 0.00% 93 Missing ⚠️
toxcore/events/group_invite.c 0.00% 92 Missing ⚠️
toxcore/events/group_peer_exit.c 18.86% 86 Missing ⚠️
toxcore/events/group_private_message.c 0.00% 86 Missing ⚠️
toxcore/events/group_custom_packet.c 0.00% 79 Missing ⚠️
toxcore/events/group_custom_private_packet.c 0.00% 79 Missing ⚠️
toxcore/events/group_peer_name.c 0.00% 79 Missing ⚠️
toxcore/events/group_topic.c 0.00% 79 Missing ⚠️
toxcore/tox_dispatch.c 56.90% 78 Missing ⚠️
... and 33 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2392      +/-   ##
==========================================
- Coverage   71.11%   66.78%   -4.34%     
==========================================
  Files         127      146      +19     
  Lines       28517    30279    +1762     
==========================================
- Hits        20280    20221      -59     
- Misses       8237    10058    +1821     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iphydf iphydf force-pushed the ordered-events branch 3 times, most recently from 9681dc9 to db7ce08 Compare January 10, 2024 23:32
Copy link
Member

@Green-Sky Green-Sky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

been running the code in multiple revisions since april last year.

@iphydf iphydf force-pushed the ordered-events branch 13 times, most recently from 646171e to 6d63467 Compare January 12, 2024 14:53
@iphydf iphydf force-pushed the ordered-events branch 3 times, most recently from f46323e to 73d9b84 Compare January 13, 2024 17:40
freylax pushed a commit to nodecum/c-toxcore that referenced this pull request Jan 13, 2024
These are quite expensive, because they go through all events to index
in a typed array that no longer exists. Clients should index in the
union array and find the event they want themselves, or use dispatch.
@iphydf iphydf merged commit b4d8826 into TokTok:master Jan 15, 2024
52 of 56 checks passed
@iphydf iphydf deleted the ordered-events branch January 15, 2024 17:38
This pull request was closed.
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.

2 participants