att / rcloud

Collaborative data analysis and visualization
http://rcloud.social
MIT License
431 stars 142 forks source link

put limits on content in the session pane #1997

Open s-u opened 8 years ago

s-u commented 8 years ago

Once the session pane accumulates too much output the browser becomes very laggy (it won't respond for many seconds) and will crash eventually. It may be desirable to limit the amount of content in the session pane by flushing old content or some such strategy. It is in particular problematic due to (lack of) #1734 which would at least allow manual intervention.

gordonwoodhull commented 8 years ago

Infinite scrolling.

It would be difficult in the notebook pane but with the completely static content of the session pane it should be easy to test a few libraries and use one.

shaneporter commented 8 years ago

How would infinite scrolling benefit here? New content is automatically appended to the bottom of the session pane, right?

Are you suggesting that as new content is appended, some old content is removed from the pane? And that this content is available to show again, should the user scroll back up?

Infinite scrolling works because it appends content, and the user has to continue scrolling down to see the new content; their current position isn't interrupted by the new content.

Surely scrolling up to see previous content would disorientate the user?

gordonwoodhull commented 8 years ago

There are different implementations of infinite scrolling.

Sure, the cheap way is just to keep appending stuff. But I'm thinking of what data tables and editors do where they only instantiate the DOM elements for the stuff that's on the screen. As you scroll they are adding and removing DOM elements on either side.

In fact, one implementation could be just to use ACE since I know they handle this correctly. There might still need to be some limit but I don't think so. I've pasted multiple copies of moby dick and war and peace into assets before.

I don't understand your objections about scrolling up and down, but maybe I've got my directions reversed. It should definitely look just like you are just scrolling through sequential content; I'm not proposing anything reversed.

(Talking about scrolling up and down is inherently ambiguous because scrollbars, mousewheels, and touchpads all interpret up and down differently.)

shaneporter commented 8 years ago

I wasn't objecting per se. I didn't fully comprehend the intention.

gordonwoodhull commented 8 years ago

I haven't tried it, but it looks like Infinity.js allows variable-height items, as long as the height is not set by CSS (?) and as long as the height doesn't change.

It is also slow to remove items. We currently allow removing error messages but maybe we don't need it once we have #1734 clearing the session pane.

Anyway, this may introduce instability, so it's probably better to attack it in 1.7.

shaneporter commented 8 years ago

That looks good.

gordonwoodhull commented 8 years ago

Confirmed that the session pane will get laggy after "only" a million short rows (with rcloud.session.log(1:1000000)), and will stop printing before another million.

shaneporter commented 8 years ago

It looks as though infinity.js doesn't work 'out of the box' for the session pane because the 'container' has a height.

I know the rcloud.session.log(1:1000000) isn't a typical scenario, but splitting on new line and appending that many individual items is very slow indeed.

The only examples with infinity that I can get working use the body as the container; in that scenario, things seem to work fine, but try it with a div with a height, and things don't work.

gordonwoodhull commented 8 years ago

Looks like this is the related issue: https://github.com/airbnb/infinity/issues/14

Looks like there might be a fork with a fix, but it also looks like they just deprecated the library (and then accepted some PRs, weird).

shaneporter commented 8 years ago

That is odd; where do you stand on the fork? Test and include if it is deemed to do what we want?

gordonwoodhull commented 8 years ago

Sure, if it works let's use it! We distribute all of our dependencies (and we don't try to track them with npm) so it's not a big deal to use a non standard version.

gordonwoodhull commented 7 years ago

PR #2307 didn't help enough, so I didn't merge it.

From my comment there:

We'll have to find another library that only generates elements when they're needed (and preferably removes them when they're not visible).

It's not documented and they call it experimental, but the infinite scrolling feature of iScroll seems to work this way.

It looks pretty simple - if you check out the source of the demo, it just has an undocumented dataset callback that retrieves a range of data. We don't need any of the ajax stuff, but we would need to store an array of output strings and supply it in response to dataset.

Holding onto many megabytes of data in JavaScript isn't a problem; it's just creating a lot of DOM elements that runs into trouble.

It looks like this library is also actively maintained.

gordonwoodhull commented 4 years ago

Putting a limit on the buffer size didn't help much. It's still really easy to completely crash the browser with lots of debug output.

On my computer this works, but reducing the sleep time below 0.01s will crash the browser:

for(i in 1:10000) {
  rcloud.session.log(paste('line', i, '\n'))
  Sys.sleep(0.01)
}