Ember/V8: Slow EmberError constructor

[schuay]

I heard through the grapevine that error handling is very slow when using current V8 versions (e.g. 6.0).

Looking at the EmberError constructor [0], the main offender is probably the call to Error.captureStackTrace. captureStackTrace used to format the stack trace lazily on access to the stack property (like builtin error objects), but since about a year it formats traces eagerly. That could be fairly expensive, and would explain a sudden slowdown in error handling code. We're looking into re-enabling lazy formatting in Error.captureStackTrace [1] soon.

I also noticed that each call to the EmberError constructor collects the stack trace three times:

1. In the call to super()
2. In the explicit Error.call(this, message)
3. In Error.captureStackTrace.

Maybe it'd be possible to avoid some of this useless work? In current V8 versions, it should be enough to just call super(message) - this will also filter to relevant frames (as Error.captureStackTrace does).

[0]

ember.js/packages/ember-debug/lib/error.js

Line 32 in 0947407

Error.captureStackTrace(this, EmberError);

[1] https://crbug.com/v8/5962

cc @hashseed, @bmeurer, @stefanpenner

[stefanpenner]

@schuay I believe our usage of Error.captureStackTrace are purely for cosmetic reasons. I'll just remove it.

[stefanpenner]

• remove captureStackTrace: [BUGFIX beta] remove Error.captureStackTrace #15519
• remove duplicate "super" call super() and Error.call(this, message) but this will require a quick test to ensure we don't foobar anything. [BUGFIX release] Fix Ember.Error inheritance #15522

[stefanpenner]

In the call to super()
In the explicit Error.call(this, message)

is kinda funny bit-rot. Time to clean that up.

[stefanpenner]

We also do stack trace stuff herE: https://github.com/emberjs/ember.js/blob/master/packages/ember-debug/lib/deprecate.js#L81-L118

When we provide deprecations. These get hit quite abit, I wonder if we should log a deprecation sans stack, but provide via console message how to enable stacks...

Thoughts @rwjblue ?

[schuay]

I'm a bit confused by

ember.js/packages/ember-debug/lib/deprecate.js

Line 98 in 23a8c92

// Chrome

. The 'Chrome' branch seems like dead code, at least on current versions. There's no 'arguments' property in error objects.

It might also be easier for you to use Error.prepareStackTrace (V8 only) to extract relevant info rather than expensive RegExp.p.replace calls. See https://github.com/v8/v8/wiki/Stack-Trace-API.

[schuay]

By the way, Error.captureStackTrace now formats lazily again: https://chromium-review.googlesource.com/c/574530/

[stefanpenner]

I'm a bit confused by

ember.js/packages/ember-debug/lib/deprecate.js

Line 98 in 23a8c92

// Chrome

. The 'Chrome' branch seems like dead code, at least on current versions. There's no 'arguments' property in error objects.

Ya that code does look funky. I suspect needs some TLC, looking now

[stefanpenner]

When we provide deprecations. These get hit quite abit, I wonder if we should log a deprecation sans stack, but provide via console message how to enable stacks...

LIkely just via the ENV.LOG_STACKTRACE_ON_DEPRECATION

[stefanpenner]

@schuay Error.prepareStackTrace doesn't seem available..

[stefanpenner]

@schuay Error.prepareStackTrace doesn't seem available..

Ah this method, we must implement. I forgot about it :P

[stefanpenner]

It might also be easier for you to use Error.prepareStackTrace (V8 only) to extract relevant info rather than expensive RegExp.p.replace calls. See https://github.com/v8/v8/wiki/Stack-Trace-API.

thoughts on #15524 ?

[pixelhandler]

@schuay @stefanpenner is this still an issue, perhaps we should close, what do you think?

[schuay]

I don't know what the current status is - have all fixes landed?

[stefanpenner]

I believe they have not landed, there were some strange test failures and I simply didn't have the time to debug them.

[locks]

I'm closing due to inactivity. If this is still relevant, please reopen!