search

Ecotrust / cogs-priorities

national-scale refuge planning tool
Other
1 stars 1 forks source link

Remove planning units table #21

Closed perrygeo closed 10 years ago

perrygeo commented 10 years ago

Perhaps keep as separate html page?

rhodges commented 10 years ago

If we don't keep the table as a separate page - check out branch "remove-table" and commit c0d3daa297586009b041ac0edc76099d43f6b084

That reduces the response from ~55k to ~6k in my tests. More testing required and more code review to make sure nothing uses that "best" part of the response and that this fix is golden.

removed parts could be re-implemented separately so we don't carry all of that baggage in the responses when we don't need it.

perrygeo commented 10 years ago

Looks good. We could probably even remove that entire loop around line 619 (for pu in bestpus:) as it's only function appears to be calculating information for the planning units report.

Could you give that a shot and profile/time the function before/after to see if it gives us a performance enhancement as a nice side effect? Might be good to profile the memory usage as well as time to see if we can close #26

rhodges commented 10 years ago

Fixed: 5ba09a8126ce482333bd6cbc503b7b23e3578b07

We did not retain the ability to view the table elsewhere. Performance for this aspect increased to: 3% of time for test cases (down 97%) 11% of size of results response for test cases (down 89%) 4% of memory use for test cases (down 96%)

I cannot close #26 at present as I'm still seeing huge memory loads in my stage environment. Will continue to investigate.