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

Mitigated Serial Monitor resource exhaustion #2491

Merged
merged 4 commits into from Dec 29, 2014

Conversation

@cmaglie
Copy link
Member

commented Dec 10, 2014

Fixes #2233

Tested on a Dual-core 1.8Ghz Linux 64bit system with 4GB of RAM: the Serial Monitor performance are dramatically improved.

/cc @PaulStoffregen @willie68 @xxxajk @ffissore

len = getDocument().getLength();
if (len > maxChars) {
int n = len - maxChars;
System.out.println("trimDocument: remove " + n + " chars");

This comment has been minimized.

Copy link
@ffissore

ffissore Dec 10, 2014

Contributor

Left over

This comment has been minimized.

Copy link
@cmaglie

cmaglie Dec 10, 2014

Author Member

Fixed

@PaulStoffregen

This comment has been minimized.

Copy link
Collaborator

commented Dec 10, 2014

Oh, opps, yeah, I missed that print before committing the code.

@ffissore

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2014

It looks like if I unckeck autoscrolling, it stops working. Right after re-enabling it, it says (with that removed println) it has removed all the accumulated extra chars. Something like

trimDocument: remove 200000 chars
... disable autoscroll and wait a couple of minutes, then reenable it ...
trimDocument: remove 8000000 chars
@PaulStoffregen

This comment has been minimized.

Copy link
Collaborator

commented Dec 12, 2014

Yup, work remains to be done for turning off autoscroll. I wrote a more detailed explanation here:

#2233 (comment)

@PaulStoffregen

This comment has been minimized.

Copy link
Collaborator

commented Dec 12, 2014

If you or anyone else reading works on this, please understand that you absolutely must not trim the document while autoscroll is turned off. Doing so ruins the user experience. But you can discard incoming data (before it's added to the document), hopefully after some reasonable threshold that won't be hit for slower data rates and only brief disable of autoscroll.

cmaglie added 2 commits Dec 23, 2014
Before this patch every byte received from Serial
invokes a String allocation, not really efficient.

Moreover a InputStreamReader is chained on the serial
InputStream to correctly convert bytes into UTF-8
characters.
When the "autoscroll" checkbox is deselected the buffer may continue
to grow up to twice of the maximum size.

This is a compromise to ensure a better user experience and, at the
same time, reduce the chance to lose data and get "holes" in the
serial stream.

See #2491
@cmaglie

This comment has been minimized.

Copy link
Member Author

commented Dec 23, 2014

I've updated the pull request:

  • I've found (and fixed) the performance killer here: cmaglie@63f5d26#diff-4ed4a5bd21c427b12e6ebdb4f9f57e4aL273
    as you can see a new string was allocated for every character received. This has (incidentally?) already been fixed in 1.6.0 when we moved to JSSC. Now the CPU usage dropped from 100 to 20!
  • The autoscroll limit has been implemented as suggested by @PaulStoffregen.
@cmaglie cmaglie merged commit 8e0a311 into arduino:master Dec 29, 2014
@cmaglie cmaglie deleted the cmaglie:serial-monitor-oom branch Dec 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.