Skip to content

autoupdater: update download progress display less frequently #274

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

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

Conversation

neocturne
Copy link
Member

Updating the "XXXXX / XXXXX KiB" display for every run of recv_image_cb() is a significant bottleneck on slow consoles. This was reported for a device using a 9600 Baud serial console, but the slowdown is noticeable even on a 115200 Baud console, especially when the network is fast.

Replace the output with a "XX.X / XX.X MiB" display and only update it every 0.1 MiB to fix the issue.

Closes #273

Updating the "XXXXX / XXXXX KiB" display for every run of
recv_image_cb() is a significant bottleneck on slow consoles. This was
reported for a device using a 9600 Baud serial console, but the slowdown
is noticeable even on a 115200 Baud console, especially when the network
is fast.

Replace the output with a "XX.X / XX.X MiB" display and only update it
every 0.1 MiB to fix the issue.

Closes freifunk-gluon#273
@rotanid
Copy link
Member

rotanid commented Apr 21, 2024

@neocturne @kpanic23 what about this, has this been tested already?

@neocturne
Copy link
Member Author

@neocturne @kpanic23 what about this, has this been tested already?

I have tested it on one of my devices with a 115200 Baud UART, but it would be great to hear how well it works at 9600 Baud.

@Djfe
Copy link

Djfe commented Mar 27, 2025

I have an Aerohive ap121 here and could test this patch

@rotanid
Copy link
Member

rotanid commented Apr 1, 2025

@Djfe that statement might be true for many, including me, but it doesn't matter at all until someone really does it :-P

@T-X
Copy link
Contributor

T-X commented Apr 28, 2025

Had a look at this code, so that would likely speed things up for @kpanic23 100x, from 20min for 44MiB = 0.30 MBit/s to 30MBit/s. Of course still potentially a bottleneck.

But this change is straightforward and I like the simplicity of it, especially for something important like the autoupdater. Therefore looks good to me.

@neocturne
Copy link
Member Author

44MiB are probably a typo, and the image is actually 4 or 4.4MiB?

With the new code we should achieve ~1.92MiB/s when logging over a 9600 Baud 8N1 UART (9600 bits per second / 10 bits per character / 50 characters per message / 10 messages per MiB). Which still seems reasonable for our typical image sizes.

The bottleneck could be avoided completely if we logged only once per fixed time interval instead of making it depend on the amount of transferred data... unsure if that is worth complicating the code.

@neocturne
Copy link
Member Author

@T-X Would you still say the solution is good enough with the numbers I provided?

@T-X
Copy link
Contributor

T-X commented Apr 28, 2025

So for an 8 MiB image at a 9600 baud console (worst case) output would need (8*10*50)/(9600/10) = 4.2 seconds. Typically, at 115200 baud, more like 0.03 seconds. Sounds negligible for now in comparison to the total update time. And I like the simplicity/readability of this change, so sounds good to me.

@rotanid
Copy link
Member

rotanid commented Apr 29, 2025

44MiB are probably a typo, and the image is actually 4 or 4.4MiB?

no typo, see #273
also, our own images for HiveAP are also ~16MB for v2023.1.x based builds and ~44 for v2021.1.x based builds.

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.

autoupdater: extremely slow image download on HiveAP
4 participants