Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent inspect from being called recursively #115

Merged
merged 1 commit into from
Mar 27, 2014

Conversation

kshahkshah
Copy link
Contributor

This is related to the problems with 2.0.0 and the Spreadsheet gem.

It appears this simple fix will prevent absurdly long load times on calls like:

spreadsheet = Roo::Excel.new("foo.xls")

as opposed to:

Roo::Excel.new("foo.xls").first_line

which has always run quite quickly.

The difference is the first version, when called in a program like IRB implicitly calls inspect, which (I think?) recursively calls inspect on all the nested instance variables, again, I think. Even files of a relatively small size (165kb xls) were taking hours to load before I made this change, now it takes a brief second.

Empact added a commit that referenced this pull request Mar 27, 2014
Prevent inspect from being called recursively
@Empact Empact merged commit f088ed0 into roo-rb:master Mar 27, 2014
@kshahkshah kshahkshah mentioned this pull request May 25, 2014
@dborin
Copy link

dborin commented Sep 30, 2014

Please reopen this. This is NOT fixed. Doing

s = Roo::Excel.new('./foo.xls')

and

s = Roo::Excel.new('./foo.xls').inspect

Result in the same insane output of reading something (unsure what since my foo.xls is two columns with three rows) that takes upwards of 5 minutes to load.

@kshahkshah
Copy link
Contributor Author

I'll take a look in the morning. What Ruby are you running?
On Sep 30, 2014 12:06 AM, "David Borin" notifications@github.com wrote:

Please reopen this. This is NOT fixed. Doing

s = Roo::Excel.new('./foo.xls')

and

s = Roo::Excel.new('./foo.xls').inspect

Result in the same insane output of reading something (unsure what since
my foo.xls is two columns with three rows) that takes upwards of 5 minutes
to load.


Reply to this email directly or view it on GitHub
#115 (comment).

@dborin
Copy link

dborin commented Sep 30, 2014

2.0.0-p481, but I saw this in 2.1.1 as well :( Creek is able to quickly
load up this file when I convert it to xlsx.

On Mon, Sep 29, 2014 at 9:28 PM, Kunal Shah notifications@github.com
wrote:

I'll take a look in the morning. What Ruby are you running?
On Sep 30, 2014 12:06 AM, "David Borin" notifications@github.com wrote:

Please reopen this. This is NOT fixed. Doing

s = Roo::Excel.new('./foo.xls')

and

s = Roo::Excel.new('./foo.xls').inspect

Result in the same insane output of reading something (unsure what since
my foo.xls is two columns with three rows) that takes upwards of 5
minutes
to load.


Reply to this email directly or view it on GitHub
#115 (comment).


Reply to this email directly or view it on GitHub
#115 (comment).

@dborin
Copy link

dborin commented Sep 30, 2014

And thanks for looking at this so quickly

@alainmeier
Copy link

+1 I am getting the same issue on 2.1.3p242

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants