Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
4 years, 2 months ago by gsathya Modified:
4 years, 2 months ago Reviewers:
Benedikt Meurer, adamk Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[promises] Move PromiseReactionJob to c++
This patch results in a 11% improvement over 5 runs in the
bluebird benchmark.
BUG=v8:5343, v8:5046
TBR=bmeurer@chromium.org
Committed: https://crrev.com/6f94a8f1c7f0a94c74c5055b02b660d8e93fe5fe
Cr-Commit-Position: refs/heads/master@{#40239}
Patch Set 1 #Patch Set 2 : remove ; #
Total comments: 3
Patch Set 3 : use context from runtime function #Patch Set 4 : get correct length #Patch Set 5 : fix whitespace #
Total comments: 32
Patch Set 6 : address review comments #Patch Set 7 : recreate exception logic #Patch Set 8 : merge ifs #Patch Set 9 : fix build #
Total comments: 16
Patch Set 10 : address review comments #Patch Set 11 : wat #Patch Set 12 : fix promisedebugeventscope #
Created: 4 years, 2 months ago
Messages
Total messages: 64 (50 generated)
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [promises] Move PromiseReactionJob to c++ BUG=v8:5343 ========== to ========== [promises] Move PromiseReactionJob to c++ BUG=v8:5343 ==========
gsathya@chromium.org changed reviewers: + adamk@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
gsathya@chromium.org changed reviewers: - adamk@chromium.org
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2406343002/diff/20001/src/js/promise.js File src/js/promise.js (right): https://codereview.chromium.org/2406343002/diff/20001/src/js/promise.js#newco... src/js/promise.js:220: function() {}); This is such a hack https://codereview.chromium.org/2406343002/diff/20001/src/objects.h File src/objects.h (right): https://codereview.chromium.org/2406343002/diff/20001/src/objects.h#newcode6715 src/objects.h:6715: class PromiseHandleInfo : public Struct { PromiseReactionInfo is a better name?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
adamk@chromium.org changed reviewers: + adamk@chromium.org
https://codereview.chromium.org/2406343002/diff/20001/src/runtime/runtime-int... File src/runtime/runtime-internal.cc (right): https://codereview.chromium.org/2406343002/diff/20001/src/runtime/runtime-int... src/runtime/runtime-internal.cc:586: reaction_context); Rather than creating a function, I think you should be able to capture the current native context here, from isolate->native_context().
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...) v8_win_nosnap_shared_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/10/11 17:56:41, adamk wrote: > https://codereview.chromium.org/2406343002/diff/20001/src/runtime/runtime-int... > File src/runtime/runtime-internal.cc (right): > > https://codereview.chromium.org/2406343002/diff/20001/src/runtime/runtime-int... > src/runtime/runtime-internal.cc:586: reaction_context); > Rather than creating a function, I think you should be able to capture the > current native context here, from isolate->native_context(). PTAL
Description was changed from ========== [promises] Move PromiseReactionJob to c++ BUG=v8:5343 ========== to ========== [promises] Move PromiseReactionJob to c++ This patch results in a 12% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343 ==========
Sorry for all the naming nits below... https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc File src/isolate.cc (right): https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3015 src/isolate.cc:3015: if (debug()->is_active()) { I think you could create a helper class that'd encapsulate this work. You could put it on the stack, passing |this|, before_debug_event, and after_debug_event, and it would take care of firing the before_debug_event at construction and the after_debug_event on destruction. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3026 src/isolate.cc:3026: if (tasks->IsJSArray()) { Can you add a comment here about the two possible states tasks might be in? https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3029 src/isolate.cc:3029: Handle<FixedArrayBase> elements(array->elements(), this); This is now unused. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3032 src/isolate.cc:3032: for (int i = 0; i < length; i += 2) { Please add a DCHECK that length is even (otherwise this loop is unsafe) https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3033 src/isolate.cc:3033: Handle<Object> argv[] = {value, accessor->Get(array, i), Please add the DCHECK for Has(i) and Has(i+1). https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3035 src/isolate.cc:3035: *result = Execution::TryCall(this, promise_handle(), I'd hoist this out of the loop, you could store it in a handle the same place where you create |value| and |tasks|. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3036 src/isolate.cc:3036: factory()->undefined_value(), Same as the above, I'd hoist this out. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3037 src/isolate.cc:3037: arraysize(argv), argv, maybe_exception); I'm not sure the same logic about result and maybe_exception holds as it did before. This is only going to remember the last call. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3092 src/isolate.cc:3092: // TODO(gsathya): remove IsJSFunction? You'd have to change the V8 API before you could do that, so I don't think this TODO belongs here. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3171 src/isolate.cc:3171: if (microtask->IsPromiseContainer()) { This should be merged with the else above https://codereview.chromium.org/2406343002/diff/80001/src/js/promise.js File src/js/promise.js (right): https://codereview.chromium.org/2406343002/diff/80001/src/js/promise.js#newco... src/js/promise.js:193: var id, name, before_debug, after_debug, instrumenting = DEBUG_IS_ACTIVE; Nit: s/before_debug/beforeDebug/ Same for after_debug https://codereview.chromium.org/2406343002/diff/80001/src/objects-debug.cc File src/objects-debug.cc (right): https://codereview.chromium.org/2406343002/diff/80001/src/objects-debug.cc#ne... src/objects-debug.cc:932: reaction_context()->ObjectVerify(); I think I led you down the wrong path here the first time (still waiting for a response on v8-dev to be sure). But I think the only checks you want here are that fields are of the right type, e.g., CHECK(tasks()->IsJSArray() || tasks()->IsCallable()) https://codereview.chromium.org/2406343002/diff/80001/src/objects.h File src/objects.h (right): https://codereview.chromium.org/2406343002/diff/80001/src/objects.h#newcode6687 src/objects.h:6687: class PromiseContainer : public Struct { This name now sounds too generic. Maybe add a TODO to rename it? https://codereview.chromium.org/2406343002/diff/80001/src/objects.h#newcode6714 src/objects.h:6714: class PromiseHandleInfo : public Struct { This name is a little funny; why not PromiseReactionJobInfo? https://codereview.chromium.org/2406343002/diff/80001/src/objects.h#newcode6718 src/objects.h:6718: DECL_ACCESSORS(deferred, Object) Isn't this guaranteed to be a JSObject? https://codereview.chromium.org/2406343002/diff/80001/src/objects.h#newcode6721 src/objects.h:6721: DECL_ACCESSORS(reaction_context, Context) I think "context" would be a fine name for this.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [promises] Move PromiseReactionJob to c++ This patch results in a 12% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343 ========== to ========== [promises] Move PromiseReactionJob to c++ This patch results in a 11% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/10582) v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...)
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc File src/isolate.cc (right): https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3015 src/isolate.cc:3015: if (debug()->is_active()) { On 2016/10/11 23:14:30, adamk wrote: > I think you could create a helper class that'd encapsulate this work. You could > put it on the stack, passing |this|, before_debug_event, and after_debug_event, > and it would take care of firing the before_debug_event at construction and the > after_debug_event on destruction. Done. But it's the same LoC https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3026 src/isolate.cc:3026: if (tasks->IsJSArray()) { On 2016/10/11 23:14:30, adamk wrote: > Can you add a comment here about the two possible states tasks might be in? Done. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3029 src/isolate.cc:3029: Handle<FixedArrayBase> elements(array->elements(), this); On 2016/10/11 23:14:30, adamk wrote: > This is now unused. Done. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3032 src/isolate.cc:3032: for (int i = 0; i < length; i += 2) { On 2016/10/11 23:14:30, adamk wrote: > Please add a DCHECK that length is even (otherwise this loop is unsafe) Done. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3033 src/isolate.cc:3033: Handle<Object> argv[] = {value, accessor->Get(array, i), On 2016/10/11 23:14:30, adamk wrote: > Please add the DCHECK for Has(i) and Has(i+1). Done. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3035 src/isolate.cc:3035: *result = Execution::TryCall(this, promise_handle(), On 2016/10/11 23:14:30, adamk wrote: > I'd hoist this out of the loop, you could store it in a handle the same place > where you create |value| and |tasks|. Done. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3036 src/isolate.cc:3036: factory()->undefined_value(), On 2016/10/11 23:14:30, adamk wrote: > Same as the above, I'd hoist this out. Done. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3037 src/isolate.cc:3037: arraysize(argv), argv, maybe_exception); On 2016/10/11 23:14:30, adamk wrote: > I'm not sure the same logic about result and maybe_exception holds as it did > before. This is only going to remember the last call. Done. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3092 src/isolate.cc:3092: // TODO(gsathya): remove IsJSFunction? On 2016/10/11 23:14:30, adamk wrote: > You'd have to change the V8 API before you could do that, so I don't think this > TODO belongs here. Done. https://codereview.chromium.org/2406343002/diff/80001/src/isolate.cc#newcode3171 src/isolate.cc:3171: if (microtask->IsPromiseContainer()) { On 2016/10/11 23:14:30, adamk wrote: > This should be merged with the else above Done. https://codereview.chromium.org/2406343002/diff/80001/src/js/promise.js File src/js/promise.js (right): https://codereview.chromium.org/2406343002/diff/80001/src/js/promise.js#newco... src/js/promise.js:193: var id, name, before_debug, after_debug, instrumenting = DEBUG_IS_ACTIVE; On 2016/10/11 23:14:30, adamk wrote: > Nit: s/before_debug/beforeDebug/ > > Same for after_debug Done. https://codereview.chromium.org/2406343002/diff/80001/src/objects-debug.cc File src/objects-debug.cc (right): https://codereview.chromium.org/2406343002/diff/80001/src/objects-debug.cc#ne... src/objects-debug.cc:932: reaction_context()->ObjectVerify(); On 2016/10/11 23:14:30, adamk wrote: > I think I led you down the wrong path here the first time (still waiting for a > response on v8-dev to be sure). But I think the only checks you want here are > that fields are of the right type, e.g., > > CHECK(tasks()->IsJSArray() || tasks()->IsCallable()) Why is tasks special? Don't we want to check if deferred is Undefined or callable too? https://codereview.chromium.org/2406343002/diff/80001/src/objects.h File src/objects.h (right): https://codereview.chromium.org/2406343002/diff/80001/src/objects.h#newcode6687 src/objects.h:6687: class PromiseContainer : public Struct { On 2016/10/11 23:14:30, adamk wrote: > This name now sounds too generic. Maybe add a TODO to rename it? Done. https://codereview.chromium.org/2406343002/diff/80001/src/objects.h#newcode6714 src/objects.h:6714: class PromiseHandleInfo : public Struct { On 2016/10/11 23:14:30, adamk wrote: > This name is a little funny; why not PromiseReactionJobInfo? Done. https://codereview.chromium.org/2406343002/diff/80001/src/objects.h#newcode6718 src/objects.h:6718: DECL_ACCESSORS(deferred, Object) On 2016/10/11 23:14:30, adamk wrote: > Isn't this guaranteed to be a JSObject? Nope this can be UNDEFINED when we have more than one callback. The comment in PromiseReactionJob explains this. Happy to add another comment here if you think that's useful. https://codereview.chromium.org/2406343002/diff/80001/src/objects.h#newcode6721 src/objects.h:6721: DECL_ACCESSORS(reaction_context, Context) On 2016/10/11 23:14:30, adamk wrote: > I think "context" would be a fine name for this. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Looking good, almost there. Excited about the perf boost! https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc File src/isolate.cc (right): https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3013: class PromiseDebugEventHelper { We often name these things scopes, so how about PromiseDebugEventScope? https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3015: PromiseDebugEventHelper(Isolate* isolate, Handle<Object> before, You could reduce the lines-of-code (or at least encapsulate more of the logic) by taking Object* as before/after and constructing the handles 1) only if debug is active and 2) after checking debug_is_active https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3066: if (result->is_null() && maybe_exception->is_null()) { Copy and paste a comment with this, too: // If execution is terminating, just bail out. https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3074: Execution::TryCall(this, promise_handle(), factory()->undefined_value(), You can pass promise_handle_fn and undefined here https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3076: if (result->is_null() && maybe_exception->is_null()) { No need to do anything special in this case, right? We're about to return anyway. https://codereview.chromium.org/2406343002/diff/160001/src/objects-debug.cc File src/objects-debug.cc (right): https://codereview.chromium.org/2406343002/diff/160001/src/objects-debug.cc#n... src/objects-debug.cc:927: CHECK(tasks()->IsJSArray() || tasks()->IsCallable()); Aren't there more things you could check here? Basically I'd expect to see CHECKs about all the expected states of the fields. https://codereview.chromium.org/2406343002/diff/160001/src/objects.h File src/objects.h (right): https://codereview.chromium.org/2406343002/diff/160001/src/objects.h#newcode6714 src/objects.h:6714: // Struct to hold state required for PromiseReactionJobInfo. Typo: remove "Info" from end of this comment. https://codereview.chromium.org/2406343002/diff/160001/src/objects.h#newcode6730 src/objects.h:6730: static const int kContext = kAfterDebugEventOffset + kPointerSize; s/kContext/kContextOffset/
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...) v8_linux_arm_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng_trigger...)
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by gsathya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc File src/isolate.cc (right): https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3013: class PromiseDebugEventHelper { On 2016/10/12 16:18:11, adamk wrote: > We often name these things scopes, so how about PromiseDebugEventScope? Done. https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3015: PromiseDebugEventHelper(Isolate* isolate, Handle<Object> before, On 2016/10/12 16:18:11, adamk wrote: > You could reduce the lines-of-code (or at least encapsulate more of the logic) > by taking Object* as before/after and constructing the handles 1) only if debug > is active and 2) after checking debug_is_active Done. https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3066: if (result->is_null() && maybe_exception->is_null()) { On 2016/10/12 16:18:11, adamk wrote: > Copy and paste a comment with this, too: > > // If execution is terminating, just bail out. Done. https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3074: Execution::TryCall(this, promise_handle(), factory()->undefined_value(), On 2016/10/12 16:18:11, adamk wrote: > You can pass promise_handle_fn and undefined here Done. https://codereview.chromium.org/2406343002/diff/160001/src/isolate.cc#newcode... src/isolate.cc:3076: if (result->is_null() && maybe_exception->is_null()) { On 2016/10/12 16:18:11, adamk wrote: > No need to do anything special in this case, right? We're about to return > anyway. Done. https://codereview.chromium.org/2406343002/diff/160001/src/objects-debug.cc File src/objects-debug.cc (right): https://codereview.chromium.org/2406343002/diff/160001/src/objects-debug.cc#n... src/objects-debug.cc:927: CHECK(tasks()->IsJSArray() || tasks()->IsCallable()); On 2016/10/12 16:18:11, adamk wrote: > Aren't there more things you could check here? Basically I'd expect to see > CHECKs about all the expected states of the fields. Yeah I was curious about this too. I guess you missed my question in the previous reply. Done. https://codereview.chromium.org/2406343002/diff/160001/src/objects.h File src/objects.h (right): https://codereview.chromium.org/2406343002/diff/160001/src/objects.h#newcode6714 src/objects.h:6714: // Struct to hold state required for PromiseReactionJobInfo. On 2016/10/12 16:18:12, adamk wrote: > Typo: remove "Info" from end of this comment. Done. https://codereview.chromium.org/2406343002/diff/160001/src/objects.h#newcode6730 src/objects.h:6730: static const int kContext = kAfterDebugEventOffset + kPointerSize; On 2016/10/12 16:18:12, adamk wrote: > s/kContext/kContextOffset/ Done.
lgtm
gsathya@chromium.org changed reviewers: + bmeurer@chromium.org
On 2016/10/12 20:11:54, adamk wrote: > lgtm +benedikt for compiler/
Description was changed from ========== [promises] Move PromiseReactionJob to c++ This patch results in a 11% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343 ========== to ========== [promises] Move PromiseReactionJob to c++ This patch results in a 11% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343,v8:5046 ==========
Description was changed from ========== [promises] Move PromiseReactionJob to c++ This patch results in a 11% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343,v8:5046 ========== to ========== [promises] Move PromiseReactionJob to c++ This patch results in a 11% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343,v8:5046 TBR=bmeurer@chromium.org ==========
TBRing bmeurer since this is just adding a new instance type.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by gsathya@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [promises] Move PromiseReactionJob to c++ This patch results in a 11% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343,v8:5046 TBR=bmeurer@chromium.org ========== to ========== [promises] Move PromiseReactionJob to c++ This patch results in a 11% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343,v8:5046 TBR=bmeurer@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #12 (id:210001)
Message was sent while issue was closed.
Description was changed from ========== [promises] Move PromiseReactionJob to c++ This patch results in a 11% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343,v8:5046 TBR=bmeurer@chromium.org ========== to ========== [promises] Move PromiseReactionJob to c++ This patch results in a 11% improvement over 5 runs in the bluebird benchmark. BUG=v8:5343,v8:5046 TBR=bmeurer@chromium.org Committed: https://crrev.com/6f94a8f1c7f0a94c74c5055b02b660d8e93fe5fe Cr-Commit-Position: refs/heads/master@{#40239} ==========
Message was sent while issue was closed.
Patchset 12 (id:??) landed as https://crrev.com/6f94a8f1c7f0a94c74c5055b02b660d8e93fe5fe Cr-Commit-Position: refs/heads/master@{#40239}
Message was sent while issue was closed.
Very nice! LGTM |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
