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

Replace contour HashMap keys with integer tuples instead of Strings #108

Merged
merged 4 commits into from
Jun 3, 2024

Conversation

antbern
Copy link
Contributor

@antbern antbern commented Jun 2, 2024

Some small changes to improve execution time and memory usage for the xyz2contours function even further!

  • Changed the keys in the obj and curves HashMap to be tuples of (i64,i64,u8) instead of strings that needed to be formatted and split on every iteration. Since this is quite a hot code-path it improves both memory usage (needed allocations on the heap) and runtime (less operations to perform) 🚀
  • Changed to use the more efficient read_lines_no_alloc function when converting the result of the curve generation.
  • Improved file handling to not open and close the file on every iteration since lines were only appended anyways.
  • Instead of reading in all the points and storing them for later computing the average of each cell, we now only store the sum and the count. This leads to a substantial reduction in memory usage during this part of the processing. On my tests down ca 100MB 👍

@rphlo rphlo merged commit df52e7a into rphlo:master Jun 3, 2024
4 checks passed
@rphlo
Copy link
Owner

rphlo commented Jun 3, 2024

This saved a bit more than 10% (1 minute) on my usual batch execution time (now at 8m25s).

Anyway, Great job @antbern !

Do you have already something in mind for next weekend?

@antbern
Copy link
Contributor Author

antbern commented Jun 3, 2024

This saved a bit more than 10% (1 minute) on my usual batch execution time (now at 8m25s).

Wow that is amazing! On my single-file job it cut something like 4-8 seconds. Down to 1:25 mins.

Do you have already something in mind for next weekend?

Yes I do, I started replacing all the string concatenation and splitting in knolldetector with Vec and structs which seems to have improved memory usage and speed for that quite a bit (see #110 ). Also there is a place where we read the "pins.txt" file in a loop that is quadratic in the number of rows (for computing the distances between all of them) causing something like 700*700 reads of the file 😄 Better to read the entire file to memory and then to the logic IMO 🚀

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.

2 participants