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

reduce unnecessary redrawing of lines in MultiSparkLine.add_values() #69

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

Karel-Kroeze
Copy link

add_values would repeatedly call update for each line where a value was added, and update calls _draw which re-draws all lines each time it is called.

By keeping track of lines that need to be updated and only calling update once at the end of add_values, we avoid (by my best attempt at basic math) $(n-1)*n$ spurious line re-draws.

Note that tracking actually changed lines is a fairly trivial saving, the main problem was in calling update for each value, as opposed to once at the end of the loop (for >2 values or so).

`add_values` would repeatedly call `update` for each line where a value was added, and `update` calls `_draw` which re-draws _all_ lines each time it is called.

By keeping track of lines that need to be updated and only calling `update` once at the end of `add_values`, we avoid $(n-1)^2$ spurious line re-draws.
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit f90a7cb into adafruit:main Jan 17, 2024
1 check passed
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jan 23, 2024
Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306 to 1.8.0 from 1.7.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306#38 from asmagill/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k to 5.0.5 from 5.0.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#134 from AdamCummick/correct-mac-error
  > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#137 from us3r64/fix/wsgiserver
  > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#139 from us3r64/fix/socket-recv-recv_into

Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Shapes to 2.8.2 from 2.8.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Shapes#69 from Karel-Kroeze/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_miniQR to 2.1.2 from 2.1.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_miniQR#28 from jamesbowman/main
  > Merge pull request adafruit/Adafruit_CircuitPython_miniQR#26 from jamesbowman/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
@RetiredWizard
Copy link

@FoamyGuy I have to learn to look around more before posting PR's. I just noticed that the PR I just submitted literally un-does the exact changes made in this PR. Was there a different version of update_line that would handle a list of lines being passed? I can't see how this PR would have worked, I'll have to think on how to achieve the same goal this PR was going for.

@RetiredWizard
Copy link

RetiredWizard commented May 11, 2024

I've stared at the code for a while and I can't see the efficiency improvement the OP is talking about.

add_values appears to only support adding a single value per sparkline so if update_line is called within the enumerate loop it gets called once per sparkline. Moving the call out of the loop and calling it at the end will cause update_line to loop internally once per sparkline. In both cases _draw gets called once for each sparkline (it would be nice to only call _draw for the last sparkline since it apparently draws all the lines but I don't think this PR accomplishes that).

If this PR worked, it would eliminate calls for sparklines that were not being updated but the OP noted that the savings from tracking the updated lines was trivial.

I looked at two possible ways to track the updated sparklines one of them involved creating a new class method called update_lines which would take a list of sparklines to update and the second was to add an additional parameter (list of sparklines) to the existing update_line method which if supplied would take precedence over the current "line:int" parameter.

I haven't done any testing or bench marking with multiple sparklines but my feeling is that unless or until someone has a test case or project that performs poorly with the current code it's probably not worth chasing a possibly trivial performance improvement.

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