armake2 icon indicating copy to clipboard operation
armake2 copied to clipboard

Move preprocess into iterator

Open bovine3dom opened this issue 5 years ago • 1 comments

Disclaimer: I don't "get" Rust so a lot of what I do here might be mad. I probably listen to the compiler a little too much.

To my surprise, the PR does work.

Summary of changes:

  • I've moved most of the loop body in preprocess_rec out into the line_muncher function
    • I've made things mutable where it seemed prudent
  • I made an iterator, PreprocessHolder, that calls the line_muncher function and keeps track of the current line
  • I made preprocess_rec use that iterator

Motivation: https://github.com/synixebrett/HEMTT/pull/57

Even on a large config, it hasn't degraded performance:

bash-5.0$ time ~/projects/armake2/target/debug/armake2 preprocess addons/weapon_rifle_blank_c/config.cpp  > iterator_v.txt

real    0m4.193s
user    0m4.136s
sys     0m0.050s
bash-5.0$ time ~/projects/armake2/target/debug/armake2 preprocess addons/weapon_rifle_blank_c/config.cpp  > master_branch.txt

real    0m4.175s
user    0m4.111s
sys     0m0.057s

The final loop could obviously be replaced with a fold if you're so inclined.

I'd appreciate someone who knows what they're doing looking over this - the dereferences everywhere look like an anti-pattern, for example. Also, help with naming things would be useful.

I'm happy to tidy the PR up before (if?) it is merged.

bovine3dom avatar Apr 03 '19 22:04 bovine3dom

Merged @synixebrett's little sanity changes (thanks!).

bash-5.0$ time ~/projects/armake2/target/release/armake2 preprocess addons/weapon_rifle_blank_c/config.cpp  > master

real    0m0.684s
user    0m0.619s
sys     0m0.064s
bash-5.0$ time ~/projects/armake2/target/release/armake2 preprocess addons/weapon_rifle_blank_c/config.cpp  > iterated

real    0m0.655s
user    0m0.617s
sys     0m0.037s

Switched to loop for reduction and tested on "release" build. Still no slowdown.

bovine3dom avatar Apr 04 '19 16:04 bovine3dom