-
Notifications
You must be signed in to change notification settings - Fork 193
HyStart++ implementation [RFC 9406] #1840
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
base: master
Are you sure you want to change the base?
Conversation
I will look at the link failure issue, that should be simple using Visual Studio. Then prepare a PR for your branch. |
Thank you! I am not using Windows/Visual Studio and haven't enough time today to fire up my VM. |
Seems to be working now. I only had to add hystart_test.c to the picoquictest project. |
Question: how do you expect to specify "use of cubic and hystart++" in a run of |
I think the best idea is to create an additional variable to Edit: Maybe it is good idea to introduce a |
…no supports app limitation
Is this still a work in progress? |
I will give an update on this next week. Implementation is ready so far. |
We have a conflict between this and PR #1856 on the use of the "-H" flag. Also with the Hybla PR. You use it to specify an Hystart++ flag; Hybla uses it to specify Hybla parameters; PR #1856 defines it as a generic flag for carrying options of the congestion control parameter. Maybe I should merge PR #1856 first, so you can use the generic parameter to set options in various protocols. This would remove the need for adding set up functions in the general code. |
Yes, I saw it yesterday. More and more characters are used by picoquic. It's hard to find a "comprehensible" character for further options. I will change my code to use the generic CC parameters introduced in #1856. Do we want to follow a specific scheme for this CC option string? I think every CC has its own options and parameters. (e.g. We don't need the RTT0 parameter in CUBIC.) Of course, there are some in common too. (e.g. HyStart) |
For BBR, I wrote a simple parser with the syntax:
If you need a single option for Hystart, we could reserve the letter 'Y', followed by digits, or something more complex followed by column. Each Hystart user could then ask Hystart to parse the option and update the Hystart state, with the Hystart parser returning something like a pointer to the next character. |
Just wanted to let you know that I'm currently taking measurements. Everything is taking longer than expected. |
# Conflicts: # picoquic/picoquic.h # picoquic/picoquic_internal.h # picoquic/quicctx.c # picoquictest/cc_compete_test.c # picoquictest/config_test.c # picoquictest/picoquic_ns.c # picoquictest/picoquic_ns.h
The HyStart algorithm configuration has been refactored and removed as a default/global option. Algorithm settings are now handled locally within individual congestion control modules (e.g., NewReno, Cubic, and Prague) through option strings, improving code modularity and clarity. This enables better future adaptability without impacting global behavior or unrelated logic.
Introduce a new test function for Cubic congestion control with Hystart++ enabled. This test simulates a 4M vs 10M batch scenario with specific parameters, ensuring accurate evaluation of congestion behaviors.
Removed the '-H' option for enabling HyStart from the help text in `config_usage_ref.txt`. This reflects the deprecation or removal of HyStart functionality in the tool.
Consolidated HyStart++ functionality into a new shared function, `picoquic_cc_hystart_pp_test`, reducing code duplication across congestion control algorithms. Updated related logic in NewReno, Cubic, and Prague implementations to use this function. Adjusted tests and debug outputs accordingly.
Simplified the HyStart++ initialization logic by merging `picoquic_hystart_pp_reset` and `picoquic_hystart_pp_init` into a single function. Removed unnecessary debug code and unused struct fields to improve code clarity and maintainability.
The HYSTART option is no longer in use and has been removed from the codebase for clarity and maintainability. This change simplifies the picoquic_option_enum_t definition by eliminating unused entries.
This commit integrates HyStart++ into the BBR (Bottleneck Bandwidth and Round-trip propagation time) algorithm, allowing selection between HyStart, HyStart++, and disabled modes. It updates initialization, state transitions, and ACK processing to incorporate HyStart++ logic, improving startup phase behavior.```
Replaced individual slow start and hystart tests with a comprehensive hystart_test function that evaluates multiple congestion control algorithms and hystart modes. Simplified the test definitions in picoquic_t and updated related unit tests accordingly. Enhanced prague.c logic to handle hystart PP states during slow start.
Added handling for the HyStart++ algorithm during BBR state setup. Updated HyStart test logic to replace `has_loss` with `queue_delay_max`.
WORK IN PROGRESS
RFC 9406
To avoid big changes in the testbed, prevent problems in currently running research using picoquic and I also want to compare SS, HyStart and HyStart++ for my research, HyStart++ can be enabled as an option. Current HyStart implementation is still the default option.
alg_number:
0 = HyStart disabled (Slow Start)
1 = HyStart enabled (default option)
2 = HyStart++ enabled
./picoquicdemo -H <alg_number>
picoquic_set_default_hystart_algorithm(quic, alg_number)
picoquic_set_hystart_algorithm(cnx, alg_number)
HyStart++ is only implemented into CUBIC currently. I want to implement it for more CC algos and write some test cases before the code is merged into the master branch.
Will update this PR with more information and results.
#1694