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

Quic: Revamp In-flight packet tracking #4103

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

akhinvasara-jumptrading
Copy link
Contributor

@akhinvasara-jumptrading akhinvasara-jumptrading commented Feb 6, 2025

This PR abstracts the data structure used by fd_quic to track in-flight packets, and switches from a linked list to a treap. It adds a lower_bound query to the treap to aid.

test_quic_adversarial_acks can be used to compare performance of diff data structures under adversarial, heavily reordered, or well-ordered traffic. According to those results, this PR increases latency tolerably in the best case, but curbs it significantly in the worst case, particulary as the number of inflight packets scales.

Example:
Starting with 5k pkt_metas, removing 10 at a time. test_quic_adversarial describes the different cases 
linked list:
Very adversarial: 46729 us
Reasonable reordering: 724 us
Happy case: 660 us

treap
Very adversarial: 3066 us
Reasonable reordering: 2124 us
Happy case: 1754 us

/* initialize free list of packet metadata */
for( ulong j = 0; j < pkt_meta_array_sz; ++j ) {
fd_quic_pkt_meta_push_back( free, &pkt_meta_array[j] );
#define FD_QUIC_MAX_INFLIGHT_LOW_ENC 20
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe want a better way to do this?

@akhinvasara-jumptrading akhinvasara-jumptrading changed the title Adds lower_bound to treap Quic: Revamp In-flight packet tracking Feb 6, 2025
@mmcgee-jump
Copy link
Contributor

Tagging @ptaffet-jump to review the tmpl change

@mmcgee-jump mmcgee-jump closed this Feb 6, 2025
@mmcgee-jump mmcgee-jump reopened this Feb 6, 2025
@akhinvasara-jumptrading akhinvasara-jumptrading marked this pull request as ready for review February 7, 2025 15:35
@akhinvasara-jumptrading akhinvasara-jumptrading marked this pull request as draft February 10, 2025 15:49
@akhinvasara-jumptrading akhinvasara-jumptrading force-pushed the akhin/tx_data_structs branch 2 times, most recently from 3f0bc26 to 70f82a5 Compare February 10, 2025 23:51
@akhinvasara-jumptrading akhinvasara-jumptrading marked this pull request as ready for review February 10, 2025 23:53
@akhinvasara-jumptrading akhinvasara-jumptrading force-pushed the akhin/tx_data_structs branch 4 times, most recently from 7e0025e to 5d36836 Compare February 11, 2025 01:27
@akhinvasara-jumptrading akhinvasara-jumptrading marked this pull request as draft February 14, 2025 19:30
@akhinvasara-jumptrading akhinvasara-jumptrading marked this pull request as ready for review February 14, 2025 20:48
@akhinvasara-jumptrading akhinvasara-jumptrading marked this pull request as draft February 14, 2025 20:53
@akhinvasara-jumptrading akhinvasara-jumptrading marked this pull request as ready for review February 14, 2025 21:59
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.

5 participants