KoffeinFlummi / armake2

Successor to armake written in Rust
GNU General Public License v2.0
50 stars 17 forks source link

Move preprocess into iterator #26

Open bovine3dom opened 5 years ago

bovine3dom commented 5 years ago

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:

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 commented 5 years ago

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.