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

use deque instead of lists for character stacks #19

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

odrling
Copy link

@odrling odrling commented Jan 12, 2024

This is a change I made during an AOC that greatly improved performance and otherwise works just like it did with the list.

only pop and append are used, so deque is more efficient.

(the _utils import is not used in the file)
@zmbc
Copy link
Owner

zmbc commented Jan 13, 2024

Cool! Thanks for your contribution -- I did not know this existed in Python.

A few questions:

  • Can you update the tests as well?
  • What is an AOC?
  • How much of a performance improvement did you see with this, and on what task?

@odrling
Copy link
Author

odrling commented Jan 13, 2024

Can you update the tests as well?

Sure, I didn't expect them to be affected, but it seems trivial to update them.

What is an AOC?

https://adventofcode.com/, sometimes I like to start it with a weird language or at least a silly idea (and never finish all the problems or I would waste too much time on the bit)

How much of a performance improvement did you see with this, and on what task?

I remember it being quite significant but I'll try to dig out the programs I wrote to back it up with some cold numbers later today.

@odrling
Copy link
Author

odrling commented Jan 13, 2024

This is where I'm testing this https://github.com/odrling/aoc/tree/25095ad64bf2f57f4213a27ba5c93e15ee27041d/03
And the related advent of code page: https://adventofcode.com/2022/day/3

There's an input generated by the website that I've pasted into input_1.
I install shakespearelang and my fork with pipx and run the program with shakespeare run solution_2.spl < input_1.
I use hyperfine as a benchmarking tool, most importantly it analyses the results and would be able to tell if the change in performance is statically significant (assuming we don't have other programs trying to hog the CPU while it's running).

With shakespearelang 1.0.0:

odrling@suisei aoc/03 on 2022 
❯ pipx install -f shakespearelang
Installing to existing venv 'shakespearelang'
  installed package shakespearelang 1.0.0, installed using Python 3.12.1
  These apps are now globally available
    - shakespeare
done! ✨ 🌟 ✨
odrling@suisei aoc/03 on 2022 
❯ hyperfine 'shakespeare run solution_2.spl < input_1'
Benchmark 1: ./run_2.sh
  Time (mean ± σ):      7.681 s ±  0.106 s    [User: 7.617 s, System: 0.061 s]
  Range (min … max):    7.577 s …  7.898 s    10 runs

With shakespearelang from my branch:

odrling@suisei aoc/03 on 2022 took 1m16s 
❯ pipx install -f git+https://github.com/odrling/shakespearelang.git@deque-stack
Installing to existing venv 'shakespearelang'
  installed package shakespearelang 1.0.0, installed using Python 3.12.1
  These apps are now globally available
    - shakespeare
done! ✨ 🌟 ✨
odrling@suisei aoc/03 on 2022 took 1m16s 
❯ hyperfine 'shakespeare run solution_2.spl < input_1'
Benchmark 1: ./run_2.sh
  Time (mean ± σ):      7.649 s ±  0.056 s    [User: 7.580 s, System: 0.065 s]
  Range (min … max):    7.552 s …  7.732 s    10 runs

So... Unlike I remembered, it does not seem to actually have an impact on the performance...

Maybe at some point I added too many things in a stack (which was most likely an error) and there it might have been significant because the list would have to grow in size, but I don't seem to actually have a good example to show a difference in performance. And it does seem to work quite as well with a list in this case.

@odrling
Copy link
Author

odrling commented Jan 13, 2024

I've updated the tests to work with deques but at this point I have nothing to show regarding possible performance improvements with this PR.

@zmbc
Copy link
Owner

zmbc commented Jan 15, 2024

@odrling Ah, I have messed around with Advent of Code a bit but didn't recognize it by its acronym 😄

Thanks for measuring! I wonder if an example could be constructed that would demonstrate the difference more clearly. I think in general, I am in favor of this change, but I'd like to bundle it with more performance improvements, starting by profiling the hotspots of actual Shakespeare programs. I might use yours as examples! It's so cool to see non-trivial Shakespeare plays "in the wild" 😃

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.

None yet

2 participants