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

"SCRIPT28: Out of stack space" in dc.js flattenStack() function (2.0.0-beta.6) #909

Closed
simon-stealthbits opened this issue Apr 9, 2015 · 13 comments

Comments

@simon-stealthbits
Copy link

The code change in flattenStack (dc.js line 3261) to support orderedGroups causes my charts to blow up with either immediate "SCRIPT28: Out of stack space" errors (in ie11) upon the call to renderAll(), or intermittently to work but then have really noticeably bad performance when repositioning a filter brush.

I stepped through with code and problem appears to be related to initial calculation of the yAxis in a call to flattenStack.

The old code was: (I was using 2.0.0-beta.1)

    function flattenStack() {
        return _chart.data().reduce(function (all, layer) {
            return all.concat(layer.values);
        }, []);
    }

the new code is: (after upgrade to 2.0.0-beta.6)

    function flattenStack() {
        var valueses = _chart.data().map(function (layer) { return layer.values; });
        var flattened = Array.prototype.concat.apply([], valueses);
        return _chart._computeOrderedGroups(flattened);
    }

I altered dc.js to revert just this one function to the beta.1 code and the stack overflow and performance issues went away.

@gordonwoodhull
Copy link
Contributor

Thanks @simon-stealthbits for the report. It does seem dumb to flatten out all the data just to get the list of keys, and I'm sure this can be improved. Any hints you can give about the shape of your data will help me troubleshoot (lots of stacks? lots of points?)

@simon-stealthbits
Copy link
Author

Thanks @gordonwoodhull for the prompt response. It's only a couple thousand (2003) rows of data, displayed as a stacked line chart and a bar chart (with the bar chart filtering the line chart).

I've put together a minimal code sample which reproduces the issue as a Gist, and uploaded the sample data used by the code sample as another Gist. I'm new to Gist and still not entirely sure how to stitch the two together to host it "live" via github, so you may need to tweak the code file.

Code sample: https://gist.github.com/simon-stealthbits/facc0c0d357a12356abe
Data sample: https://gist.github.com/simon-stealthbits/59a1ed49693aadbd9d64

@gordonwoodhull
Copy link
Contributor

Thanks, this is very helpful. I should be able to create a test case from this so it doesn't regress. I am surprised that this code invokes recursion for the 2003 points, but that must be what's happening. Well, I shouldn't speculate until I repro it in IE.

@simon-stealthbits
Copy link
Author

When the error occurs no charts are rendered. But, as I mentioned it's intermittent, having a breakpoint inside the flattenStack() method sometimes allowed the code to run and render the charts (unresponsively).

The actual stack overflow occurrs in crossfilter.js 1.3.11 - function quicksort(a, lo, hi) - on line 184:

e4 = a[i4], x4 = f(e4),

My guess is that there is something weird going on with the data being sorted though, rather than with crossfilter.

The dimension is pretty straightforward, and the grouping performs a commonplace aggregation of multiple rows into an object with 3 values, which are then displayed as stacked areas.

If I can be any further help please let me know.

@gordonwoodhull
Copy link
Contributor

Reproduced in a fiddle. Only crashes on IE11, not Chrome/Safari/Firefox.

http://jsfiddle.net/toavm85r/

@gordonwoodhull
Copy link
Contributor

@simon-stealthbits, I hope you don't mind, I added your code as a smoke test to make sure this doesn't happen again. The root cause is described here:

#766 (comment)

Fixed in 2.0.0-beta.7

gordonwoodhull added a commit that referenced this issue Apr 13, 2015
with enough data to crash crossfilter if something is wrong
with the ordering function. thanks @simon-stealthbits!
@gordonwoodhull
Copy link
Contributor

Thanks again for the report. This was a serious bug and it's great to catch it so quickly.

@xavier-d
Copy link

@gordonwoodhull,
I'm not sure to understand the patch. As with the beta 6 I was able to ordering my barCharts by using the .ordering(function(d){ return d.data.values['success'] }; ) but now with the version beta 7 in the ordering method "d" does not contains the data
d.data is undefined
Do I misunderstand something ?

@gordonwoodhull
Copy link
Contributor

Hi @xavier-d. The interface was incorrect and inconsistent with the rest of dc.js, so the default ordering function was broken.

I changed _ordinalXDomain to pass d.data instead, just the way the other charts work, so you should be able to use d.values['success'] directly now. Sorry for the changing interface but it was incorrect and causing crashes in IE.

There is a more general problem that dc.js is inconsistent on whether it passes the "inner datum" or "outer datum", however I wanted this one interface to at least be consistent. In the long run it will have to be resolved one way or the other, and a lot of stuff will break, but that won't happen until 2.1.

@xavier-d
Copy link

Understand the API change, but now with the current version we are no more able to order the ordinal stacked bar chart on a stacked value.
Do you want me to share a jsfiddle ?

@gordonwoodhull
Copy link
Contributor

There must be some other problem than what you described above.

You should be able to replace

.ordering(function(d){ return d.data.values['success'] }; )

with

.ordering(function(d){ return d.values['success'] }; )

I'm willing to fix this another way but I want to understand what is wrong. A fiddle might help, sure.

@xavier-d
Copy link

Ok, my bad.
here comes a jsfiddle working fine: http://jsfiddle.net/XavD/mvd293wb/3
In my full project I had a old crossfilter version + probably some durty code
You can probably add the jsfiddle as example to avoid other dummy questions as mine

@gordonwoodhull
Copy link
Contributor

Phew, thanks @xavier-d - I didn't think anyone would need the data processed through d3.layout.stack. I like the consistency of your example, the ordering is accessed exactly the same as the stack data:

chart.stack(testGroup,'NotExecuted',function(d) {return d.value['NotExecuted'];});
chart.ordering( function(d){return d.value['Success'];} )

Most of the time that's what people need, exactly the data as it came from the group.

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

No branches or pull requests

3 participants