search

scraperwiki / spreadsheet-download-tool

A ScraperWiki plugin for downloading data from a box as a CSV or Excel spreadsheet
BSD 2-Clause "Simplified" License
2 stars 1 forks source link

Speed up / remove or simplify colspan code #66

Open frabcus opened 10 years ago

frabcus commented 10 years ago

I'm pretty sure it is eating up CPU unnecessarily.

https://github.com/scraperwiki/spreadsheet-download-tool/issues/65

pwaller commented 10 years ago

It's structured like this so that rowspans can work. Rowspans mean that a given cell can appear in a later iteration of the loop, so we have to figure out where to insert it. This is only a linear win, and I'm not sure how much it is worth investing in getting a ~30% speed up (30% of CSV) since it means breaking rowspans.

On the other hand we could decide we don't care about rowspans, then we could throw it out. I think there might also be other approaches to rowspans which may be more efficient. But again, I don't think it is investing time in this since we can fix the load problem at another level by figuring out what to do with our customers.

frabcus commented 10 years ago

Yes to customers! On 17 Dec 2013 09:45, "Peter Waller" notifications@github.com wrote:

It's structured like this so that rowspans can work. Rowspans mean that a given cell can appear in a later iteration of the loop, so we have to figure out where to insert it. This is only a linear win, and I'm not sure how much it is worth investing in getting a ~30% speed up (30% of CSV) since it means breaking rowspans.

On the other hand we could decide we don't care about rowspans, then we could throw it out. I think there might also be other approaches to rowspans which may be more efficient. But again, I don't think it is investing time in this since we can fix the load problem at another level by figuring out what to do with our customers.

— Reply to this email directly or view it on GitHubhttps://github.com/scraperwiki/spreadsheet-download-tool/issues/66#issuecomment-30737735 .