Skip to content

Conversation

@gaearon
Copy link
Collaborator

@gaearon gaearon commented Aug 13, 2019

This measurement is already broken (mark/measure calls for it mismatch in Concurrent Mode). This makes debugging very annoying because "break on caught exceptions" stops there all the time.

It's difficult to keep it up to date because it models a pause rather than an actual work slice. This is why we keep breaking it. I propose that we just remove this timing, and fix it forward with Root Events.

@sizebot
Copy link

sizebot commented Aug 13, 2019

Warnings
⚠️

Base commit is broken: 21d6395

Generated by 🚫 dangerJS

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 13, 2019

The test warning I added uncovered a problem in production mode which is fixed by the second commit. Note that it doesn't affect open source, but it affects Ads which uses User Timing for a subset of users in prod environment.

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

This makes debugging very annoying because "break on caught exceptions" stops there all the time.

This drives me batty.

Others might want to weigh in on this too, but I'm in favor of axing it 👍

@gaearon gaearon merged commit 1fd3906 into facebook:master Aug 13, 2019
@gaearon
Copy link
Collaborator Author

gaearon commented Aug 13, 2019

@acdlite said he has a WIP replacement for this on Scheduler level anyway

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