friggog / tree-gen

Procedural generation of tree models in blender
GNU General Public License v3.0
827 stars 74 forks source link

Performance improvements #11

Closed samipfjo closed 5 years ago

samipfjo commented 5 years ago

This PR contains a slew of performance improvements, both small and large.

The largest improvement included is the replacement of deepcopy() with a simple list comprehension, which dramatically speeds up leaf generation.

Other changes include:

samipfjo commented 5 years ago

Quaking Aspen with leaves (seed 100):

Before: Stems made: 1131 in 2.632385 seconds Made 31002 leaves and 0 blossoms in 5.648526 seconds Tree generated in 8.346872 seconds

After: Stems made: 1188 in 2.405932 seconds Made 30391 leaves and 0 blossoms in 2.056793 seconds Tree generated in 4.538678 seconds


Douglas Fir with leaves (seed 100):

Before: Stems made: 5782 in 24.919251 seconds Made 354022 leaves and 0 blossoms in 55.897657 seconds Tree generated in 81.486497 seconds

After: Stems made: 6151 in 19.982167 seconds Made 351227 leaves and 0 blossoms in 15.782944 seconds Tree generated in 36.447690 seconds


Weeping Willow with leaves (seed 100):

Before: Stems made: 30326 in 94.704319 seconds Made 388967 leaves and 0 blossoms in 65.510431 seconds Tree generated in 161.144178 seconds

After: Stems made: 39770 in 124.523556 seconds Made 483994 leaves and 0 blossoms in 24.791790 seconds Tree generated in 150.350708 seconds


Summary

Stems: There appears to be a performance increase in stem generation in the Douglas Fir comparison, and a slight performance increase in the Quaking Aspen comparison. Running the numbers on the Weeping Willow comparison, however, results in an inconclusive race.

Leaves: Leaf generation is unquestionably faster. I'm so glad that deepcopy() isn't needed anymore.


Testbench specs:

Acer Aspire v3-771G (laptop) 2.4GHz Quad-Core i7-3630QM | 12GB DDR3 1600 MHz | SATA3 SSD

samipfjo commented 5 years ago

It would appear that I've messed up seeding. Hold off on merging for now.

I've figured it out; seeds are still working. By replacing rand_for_param_var() with random.uniform(-1, 1) I changed the numbers that are pumped out via a given seed. They are still consistent in the new system, they just don't sync up with the old ones.

I did notice a different bug while reading through the diff; got that patched up.

samipfjo commented 5 years ago

Tried another change. Further testing showed little difference in performance, hence the revert.

friggog commented 5 years ago

I'll try and get around to fully reviewing and testing this on the weekend as there are a lot of changes to merge without any checks! Nonetheless, very impressive work!