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

Add support to split data in chunks to reduce memory pressure #47

Open
resaldiv opened this issue Oct 22, 2020 · 4 comments
Open

Add support to split data in chunks to reduce memory pressure #47

resaldiv opened this issue Oct 22, 2020 · 4 comments

Comments

@resaldiv
Copy link

Describe the problem
When a extremely large output of text is being rendered, React cannot seem to handle the memory pressure and crashes the app. Chunking the outputs to allow react to handle them in pieces seems to relieve the memory pressure. However, this creates a problem with ansi-to-react using <code> which changes how the chunks are now rendered.

Describe the solution you'd like
If the data length passes a high threshold of character count, we split the output into chunks and then render the chunks as span using the ansi-to-react change and wrap it outside as code.

In order to do this, we will need to modify two nteract repos:

  1. outputs: add support to split the props.data in multiple chunks, map the array, and make sure to call ANSI in each iteration to render that piece of data.
  2. ansi-to-react: add support to treat each piece of chunk as a <span> element and let media plain wrap it in a <code> element.
@twavv
Copy link

twavv commented Oct 22, 2020

To do this properly, you'd need to change Anser as well, since there's no guarantee that splitting a string into chunks will happen at the "right" places (e.g., only in places where no non-default styling is active).

I also wonder if there's any lower hanging fruit in Anser to make that more memory efficient.

@resaldiv
Copy link
Author

Good call. There's a workaround for this one. If we use the same Anser for all the data chunks, the color will carry over. To do this, we could add an anser prop in the ansiToJSON function and send it from outputs. What do you think?

I've already tested this locally with some examples and it works, but if you have a specific one in mind please let me know so I can test it.

@captainsafia
Copy link
Member

@resaldiv Can you push a commit with the change your proposing and the tests? I'm having trouble visualizing how it looks ATM.

@resaldiv
Copy link
Author

@captainsafia Sure! I just added a commit with the changes in this PR.

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