Skip to content

[Scheduler] Prevent event log from growing unbounded#16781

Merged
acdlite merged 1 commit into
react:masterfrom
acdlite:scheduler-profiler-memory
Sep 13, 2019
Merged

[Scheduler] Prevent event log from growing unbounded#16781
acdlite merged 1 commit into
react:masterfrom
acdlite:scheduler-profiler-memory

Conversation

@acdlite

@acdlite acdlite commented Sep 13, 2019

Copy link
Copy Markdown
Collaborator

If a Scheduler profile runs without stopping, the event log will grow unbounded. Eventually it will run out of memory and the VM will throw an error.

To prevent this from happening, let's automatically stop the profiler once the log exceeds a certain limit. We'll also print a warning with advice to call stopLoggingProfilingEvents explicitly.

If a Scheduler profile runs without stopping, the event log will grow
unbounded. Eventually it will run out of memory and the VM will throw
an error.

To prevent this from happening, let's automatically stop the profiler
once the log exceeds a certain limit. We'll also print a warning with
advice to call `stopLoggingProfilingEvents` explicitly.
);
eventLogSize *= 2;
if (eventLogSize > MAX_EVENT_LOG_SIZE) {
console.error(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message will increase the size of the profiling build. Seems fine? Idk.

: MAX_TEST_ITERATIONS;
const maxIterations = t.logicalExpression(
'||',
t.memberExpression(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaearon Might want to review the changes I made in here. I wanted to increase the limit for a specific test so I could trigger the memory error.

@acdlite acdlite force-pushed the scheduler-profiler-memory branch 2 times, most recently from 8fe678d to 1f55c5a Compare September 13, 2019 22:25
@sizebot

sizebot commented Sep 13, 2019

Copy link
Copy Markdown
Details of bundled changes.

Comparing: 87eaa90...1f55c5a

react

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react.development.js +0.3% +0.4% 117.47 KB 117.79 KB 29.82 KB 29.95 KB UMD_DEV
react.profiling.min.js +0.7% +1.6% 15.83 KB 15.94 KB 5.9 KB 5.99 KB UMD_PROFILING
react.development.js 0.0% -0.0% 73.12 KB 73.12 KB 19.25 KB 19.25 KB NODE_DEV
React-prod.js 0.0% 0.0% 17.41 KB 17.41 KB 4.55 KB 4.56 KB FB_WWW_PROD
React-profiling.js 0.0% 0.0% 17.41 KB 17.41 KB 4.55 KB 4.56 KB FB_WWW_PROFILING

scheduler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
scheduler-unstable_mock.development.js +1.4% +2.9% 22.12 KB 22.43 KB 5.13 KB 5.28 KB UMD_DEV
scheduler-tracing.profiling.min.js 0.0% +0.1% 3.25 KB 3.25 KB 991 B 992 B NODE_PROFILING
scheduler-unstable_mock.production.min.js 0.0% -0.0% 4.73 KB 4.73 KB 1.98 KB 1.98 KB UMD_PROD
Scheduler-dev.js +1.1% +1.8% 29.7 KB 30.03 KB 7.54 KB 7.67 KB FB_WWW_DEV
Scheduler-profiling.js +1.2% +2.1% 16.84 KB 17.04 KB 3.85 KB 3.93 KB FB_WWW_PROFILING
scheduler-tracing.development.js 0.0% 0.0% 11.72 KB 11.72 KB 3.03 KB 3.03 KB NODE_DEV
scheduler-tracing.production.min.js 0.0% -0.5% 728 B 728 B 384 B 382 B NODE_PROD
scheduler-unstable_mock.development.js +1.4% +2.9% 21.93 KB 22.24 KB 5.07 KB 5.22 KB NODE_DEV
SchedulerMock-dev.js +1.5% +2.8% 22.33 KB 22.66 KB 5.17 KB 5.31 KB FB_WWW_DEV
scheduler.development.js +1.1% +1.8% 29.16 KB 29.47 KB 7.43 KB 7.57 KB NODE_DEV

Generated by 🚫 dangerJS against 1f55c5a

@sebmarkbage sebmarkbage left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this test. :) It's unnecessarily slow and binds us to this particular variant of infinite loop tracking.

@acdlite

acdlite commented Sep 13, 2019

Copy link
Copy Markdown
Collaborator Author

Ok I'm going to land as-is though because I don't have any better ideas for how to test it right now, and I'd rather have a slow test than no test

@acdlite acdlite merged commit 45898d0 into react:master Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants