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

Configurable tuning parameters #233

Merged
merged 2 commits into from
Aug 9, 2023
Merged

Configurable tuning parameters #233

merged 2 commits into from
Aug 9, 2023

Conversation

bjoernQ
Copy link
Contributor

@bjoernQ bjoernQ commented Aug 9, 2023

This adds the ability to fine-tune some parameters.

We currently use some sane defaults for a few things. However for best performance it's needed to allow changing those.
Going with Cargo features won't be a good idea so this introduces cfg-toml for configuration. I think in future we should also make the WIFI-HEAP and MTU configurable via this mechanism. But that is something for another PR.

Testing this on ESP32-C3 (suggestion). (Note: the exact values won't work the same or at all for other chips)

  • make your computer a hotspot
  • have a file named "testfile" (around 1MB should be fine) in some directory and run devserver --address <LOCALIP>:80 in that directory

Change the dns.rs example to download from your computer and extremely bump some buffers

... code after printing "Start busy loop on main"


let mut rx_buffer = [0u8; 1536 * 64];
    let mut tx_buffer = [0u8; 1536 * 1];
    let mut socket = wifi_stack.get_socket(&mut rx_buffer, &mut tx_buffer);

    loop {
        println!("Making HTTP request");
        socket.work();

        socket
            .open(IpAddress::Ipv4(Ipv4Address::new(192, 168, 137, 1)), 80)
            .unwrap();

        socket
            .write(b"GET /testfile HTTP/1.0\r\nHost: 192.168.137.1\r\n\r\n")
            .unwrap();
        socket.flush().unwrap();

        let wait_end = current_millis() + 20 * 1000;
        let mut bytes = 0;
        let t1 = current_millis();
        let mut buffer = [0u8; 1024 * 32];
        loop {
            if let Ok(len) = socket.read(&mut buffer) {
                //println!("{len}");
                bytes += len;
            } else {
                break;
            }

            if current_millis() > wait_end {
                println!("Timeout");
                break;
            }
        }
        let t2 = current_millis();

        println!();

        socket.disconnect();

        let speed = bytes as u64 * 1000 / (t2 as u64 - t1 as u64);
        println!("Bytes {}, millis {}, B/s {}", bytes, t2 - t1, speed);

        let wait_end = current_millis() + 5 * 1000;
        while current_millis() < wait_end {
            socket.work();
        }
    }

Change the IP address to match your setting. Set SSID/PASSWORD to match your computer's hotspot.

Add this as cfg.toml in the root of the workspace:

[esp-wifi]
rx_queue_size = 20
tx_queue_size = 5
static_rx_buf_num = 32
dynamic_rx_buf_num = 16
ampdu_rx_enable = 1
ampdu_tx_enable = 1
rx_ba_win = 32
max_burst_size = 8

Run the example cargo run --example dns --release --features "wifi" and see the bytes/second.

Remove the cfg.toml and re-run ... you should see a difference.

Please note: The extreme buffer sizes etc. probably won't work as is for other chips. However it shows users can change the settings.

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

LGTM! One thing I'd like to see configurable would be the frequency of the task switching; but we can do that in another PR of course :)

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Aug 9, 2023

LGTM! One thing I'd like to see configurable would be the frequency of the task switching; but we can do that in another PR of course :)

Good idea! I'll put it on my TODO list 👍

@bjoernQ bjoernQ merged commit dd87650 into main Aug 9, 2023
12 checks passed
@bjoernQ bjoernQ deleted the feature/tuning-parameters branch August 9, 2023 12:12
@bugadani
Copy link
Contributor

bugadani commented Aug 9, 2023

I'm a bit disappointed that the links point to the esp-idf docs where all the help we get (for the booleans) is "you can say if you want this or not". This is:

  • obvious
  • not at all helpful in explaining what the feature actually is

Unfortunately, this isn't the first time I've had this same exact problem with IDF documentation... Oh well, at least we have more configurability 👍

Do you think we could get configurable country info the same way?

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Aug 9, 2023

Yes - I also have no idea what those things really do t.b.h. .... If I had I had written it down 😆 At least something changes performance-wise.

But there is hope that someday someone will enlighten us in the IDF docs

Do you think we could get configurable country info the same way?

Good idea! I'll add it to my ever-growing list

Copy link
Contributor

@bugadani bugadani left a comment

Choose a reason for hiding this comment

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

I've compared to the current Kconfig and found these differences in the defaults. I wonder if these may cause some of the perf difference, but I obviously have no idea :) I wonder where the esp-wifi numbers come from, or why they are not the same as in esp-idf.

Sorry for poking my nose in this :)

static_rx_buf_num: usize,
#[default(32)]
dynamic_rx_buf_num: usize,
#[default(0)]
Copy link
Contributor

Choose a reason for hiding this comment

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

esp-idf default is 16

dynamic_tx_buf_num: usize,
#[default(0)]
ampdu_rx_enable: usize,
#[default(0)]
Copy link
Contributor

Choose a reason for hiding this comment

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

esp-idf default is 1

static_tx_buf_num: usize,
#[default(32)]
dynamic_tx_buf_num: usize,
#[default(0)]
Copy link
Contributor

Choose a reason for hiding this comment

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

esp-idf default is 1

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Aug 9, 2023

I've compared to the current Kconfig and found these differences in the defaults. I wonder if these may cause some of the perf difference, but I obviously have no idea :) I wonder where the esp-wifi numbers come from, or why they are not the same as in esp-idf.

Interesting - I think I took the defaults from ESP-IDF when I did the initial implementation ... maybe they were different back then or I just failed at the attempt

Good to know anyway. We should test on all targets with these defaults. Thanks for pointing this out

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.

3 participants