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

util: improve inspect performance #14881

Closed
wants to merge 6 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Aug 17, 2017

I reworked a couple of things in util inspect for performance. The code is fully covered.

EDIT: Updated the comment here and benchmarks.

 util/format.js type="no-replace" n=2000000                                1676.61 %        *** 6.038361e-60
 util/inspect-array.js type="denseArray_showHidden" len=100000 n=1000        87.89 %        *** 3.315801e-79
 util/inspect-array.js type="denseArray" len=100000 n=1000                   68.83 %        *** 5.648717e-52
 util/inspect-array.js type="mixedArray" len=100000 n=1000                   21.22 %        *** 2.438066e-52
 util/inspect-array.js type="sparseArray" len=100000 n=1000                   6.87 %        *** 9.551672e-50
 util/inspect-proxy.js n=1000000                                            418.34 %        *** 5.193464e-71
 util/inspect.js option="colors" method="Array" n=2000000                   121.92 %        *** 1.136217e-39
 util/inspect.js option="colors" method="Date" n=2000000                    294.42 %        *** 3.899375e-18
 util/inspect.js option="colors" method="Error" n=2000000                   507.31 %        *** 6.416333e-18
 util/inspect.js option="colors" method="Object_deep_ln" n=2000000          257.20 %        *** 8.804036e-23
 util/inspect.js option="colors" method="Object_empty" n=2000000            704.27 %        *** 8.422940e-46
 util/inspect.js option="colors" method="Object" n=2000000                  372.04 %        *** 3.430467e-19
 util/inspect.js option="colors" method="Set" n=2000000                     443.65 %        *** 7.503642e-30
 util/inspect.js option="colors" method="String_boxed" n=2000000            458.19 %        *** 4.199961e-23
 util/inspect.js option="colors" method="String_complex" n=2000000          875.46 %        *** 4.377511e-42
 util/inspect.js option="colors" method="String" n=5000000                 2209.03 %        *** 3.336942e-23
 util/inspect.js option="colors" method="TypedArray_extra" n=2000000        269.77 %        *** 6.353860e-22
 util/inspect.js option="colors" method="TypedArray" n=2000000              244.82 %        *** 1.157433e-47
 util/inspect.js option="none" method="Array" n=2000000                     298.39 %        *** 6.163207e-35
 util/inspect.js option="none" method="Date" n=2000000                      308.41 %        *** 3.828139e-26
 util/inspect.js option="none" method="Error" n=2000000                     499.46 %        *** 2.519987e-17
 util/inspect.js option="none" method="Object_deep_ln" n=2000000            470.60 %        *** 7.069515e-27
 util/inspect.js option="none" method="Object_empty" n=2000000              703.54 %        *** 5.821877e-41
 util/inspect.js option="none" method="Object" n=2000000                    678.48 %        *** 1.752143e-21
 util/inspect.js option="none" method="Set" n=2000000                       739.62 %        *** 5.402939e-18
 util/inspect.js option="none" method="String_boxed" n=2000000              483.94 %        *** 2.756513e-37
 util/inspect.js option="none" method="String_complex" n=2000000           1082.41 %        *** 3.900304e-64
 util/inspect.js option="none" method="String" n=5000000                   3954.65 %        *** 1.558337e-26
 util/inspect.js option="none" method="TypedArray_extra" n=2000000          410.59 %        *** 1.364578e-19
 util/inspect.js option="none" method="TypedArray" n=2000000                387.47 %        *** 1.789663e-23
 util/inspect.js option="showHidden" method="Array" n=2000000               272.92 %        *** 2.332172e-29
 util/inspect.js option="showHidden" method="Date" n=2000000                307.75 %        *** 4.581806e-16
 util/inspect.js option="showHidden" method="Error" n=2000000               254.46 %        *** 1.552997e-19
 util/inspect.js option="showHidden" method="Object_deep_ln" n=2000000      422.07 %        *** 4.184839e-46
 util/inspect.js option="showHidden" method="Object_empty" n=2000000        665.61 %        *** 2.095402e-24
 util/inspect.js option="showHidden" method="Object" n=2000000              626.02 %        *** 6.570478e-39
 util/inspect.js option="showHidden" method="Set" n=2000000                 815.83 %        *** 3.634105e-32
 util/inspect.js option="showHidden" method="String_boxed" n=2000000        680.82 %        *** 3.104074e-27
 util/inspect.js option="showHidden" method="String_complex" n=2000000     1058.45 %        *** 5.543800e-49
 util/inspect.js option="showHidden" method="String" n=5000000             3473.09 %        *** 6.496121e-29
 util/inspect.js option="showHidden" method="TypedArray_extra" n=2000000    345.93 %        *** 9.418444e-36
 util/inspect.js option="showHidden" method="TypedArray" n=2000000          316.96 %        *** 4.742310e-35

The main optimizations are

  • Removed visibleKeys
  • Removed proxy cache
  • Removed Object.assign
  • No key concatenating anymore
  • No key recalculating anymore
  • Improved indentation logic
  • Improved string escape logic
  • Added many fast paths
  • Optimized code branches a lot
  • Optimized (boxed) primitive handling
  • Inline code if possible
  • Only check extra keys if necessary

Besides that there were lots of smaller optimizations, the
code got a bit cleaned up and a few more tests got in.

Also two bug fixes came.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

util, benchmark

@BridgeAR BridgeAR added benchmark Issues and PRs related to the benchmark subsystem. blocked PRs that are blocked by other issues or PRs. performance Issues and PRs related to the performance of Node.js. util Issues and PRs related to the built-in util module. labels Aug 17, 2017
@BridgeAR BridgeAR mentioned this pull request Aug 17, 2017
4 tasks
lib/util.js Outdated
formatter = formatArray;
} else if (isMapIterator(value)) {
Copy link

@andrasq andrasq Aug 17, 2017

Choose a reason for hiding this comment

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

a long series of if - else if - else if statements can also be formatted as a switch (true) { ... } (note that switch does a strict comparison, ie matches not truthy but true). Strictly a layout issue, otherwise completely equivalent, both O(n) in the number of tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

I you prefer that I can change it, otherwise I would just keep it as is.

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

This is great work, thank you!

lib/util.js Outdated
if (ctx.seen === undefined) {
ctx.seen = [];
// A customInspect could be set only for partial objects.
// To prevent a breaking change, fill the lazy added array with all
Copy link
Member

Choose a reason for hiding this comment

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

grammar nit: lazy → lazily

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

lib/util.js Outdated
const customInspect = isCustomInspect(ctx, value);
if (customInspect !== undefined) {
if (ctx.seen === undefined) {
ctx.seen = [];
Copy link
Member

Choose a reason for hiding this comment

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

ctx.seen = [...ctx.seenSet]?

Also, I, personally, would be fine with having seen just be a getter/setter pair.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

lib/util.js Outdated
@@ -300,12 +298,9 @@ function stylizeNoColor(str, styleType) {

function arrayToHash(array) {
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this could just be turned into a Set as well

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that and it was actually a bit slower in my benchmarks. When 6.1 lands I might have another look at it.

lib/util.js Outdated
@@ -318,6 +313,21 @@ function ensureDebugIsInitialized() {
}


function isCustomInspect(ctx, value) {
Copy link
Member

Choose a reason for hiding this comment

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

This really sounds like it would return a boolean ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

lib/util.js Outdated
}
}

var type = typeof value;
Copy link
Member

Choose a reason for hiding this comment

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

const? Or maybe just keep it inlined, I think JS engines might be better at that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, I think it is nicer not to write typeof x many times over and type is currently redefined in line 531. But I can change it if you want me to.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah V8 has explicit inlining for the form typeof x === 'type' -- but only for that form.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not about the performance. I think it is just nicer to read not having lots of typeof when this it is not necessary. But I guess you would like me to change that?

Copy link
Member

Choose a reason for hiding this comment

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

I mean, I don’t really have strong feelings about this, but yeah, I would say storing the result of typeof is kind of unidiomatic…

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

lib/util.js Outdated
for (i = 0; i < keys.length; i++) {
const key = keys[i];
// Skip positive numbers
if (+key >= 0 === false) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not !(+key >= 0) or just +key < 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

I like the short versions but I try to write more explicit JS and the latter does not work for keys other then numbers. So if you do not feel strongly about it, I would just keep it as is?

Copy link
Member

Choose a reason for hiding this comment

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

Your call, but, like, I do find ((some inequality expression) === false) a bit odd.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it to !(+key >= 0)

lib/util.js Outdated
}
}
return output;
}


function formatTypedArray(ctx, value, recurseTimes, visibleKeys, keys) {
function formatTypedArray(ctx, value, recT, visibleKeys, keys) {
Copy link
Member

Choose a reason for hiding this comment

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

Wasn’t this a bit more readable before?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed

@addaleax
Copy link
Member

Also, sorry for the merge conflict, I fixed Evan’s nit while landing my PR.

@BridgeAR
Copy link
Member Author

@addaleax I addressed your comments and don't worry about the conflicts, I was aware of those changes and I'll rebase after #14880 lands :)
Those are minor changes anyway.

lib/util.js Outdated
}
// Using an array here is actually better for the average case then using
// a Set. `seen` will only check for the depth and will never grow to large.
if (ctx.seen.includes(value))
Copy link
Member Author

Choose a reason for hiding this comment

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

I reverted my change to a Set here: the seen part will only contain up to ctx.depth + 1 elements and in the regular case that is three. Using a Set for such a low amount of elements seems unnecessary, as it increases the code complexity due to having to handle both, an array and a set in case customInspect is set.

@BridgeAR
Copy link
Member Author

BridgeAR commented Aug 21, 2017

I rebased and added a new array fast path. So PTAL

Just for fun - this is how the profiling looks like with Array(1e5).fill(1)

 [Summary]:
   ticks  total  nonlib   name
    261    0.6%    0.6%  JavaScript
  39502   94.0%   94.3%  C++
   1797    4.3%    4.3%  GC
    129    0.3%          Shared libraries
   2147    5.1%          Unaccounted

 [C++ entry points]:
   ticks    cpp   total   name
  37497   99.2%   89.2%  v8::internal::Runtime_ObjectKeys(int, v8::internal::Object**, v8::internal::Isolate*)
     75    0.2%    0.2%  v8::internal::Builtin_ObjectGetOwnPropertyDescriptor(int, v8::internal::Object**, v8::internal::Isolate*)

0.6 % JavaScript 🤣

@BridgeAR
Copy link
Member Author

Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

1 nit otherwise LGTM

lib/util.js Outdated
@@ -211,28 +211,28 @@ function debuglog(set) {
* @param {Object} obj The object to print out.
* @param {Object} opts Optional options object that alters the output.
*/
/* legacy: obj, showHidden, depth, colors*/
function inspect(obj, opts) {
function inspect(obj, opts, /* legacy */depth, /* legacy */colors) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not worth it.

  1. changes util.inspect.length
  2. drives my IDE crazy

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I do not have a strong opinion.
I went ahead and changed it back to a similar way as before to make you happy ;-)

@BridgeAR
Copy link
Member Author

So I pushed a bit more code and updated the benchmarks. The result is pretty decent now.

@BridgeAR
Copy link
Member Author

Ok, the new benchmarks are outdated again as I was able to improve some more things that had significant impact on showHidden. So just think of most showHidden results +100%.

@BridgeAR BridgeAR added the wip Issues and PRs that are still a work in progress. label Aug 23, 2017
@BridgeAR BridgeAR removed blocked PRs that are blocked by other issues or PRs. wip Issues and PRs that are still a work in progress. labels Aug 24, 2017
@BridgeAR
Copy link
Member Author

So after looking at it again I added some final changes. The benchmarks are updated in the main part. As the code still changed significantly, please take a close look again. @addaleax @refack @andrasq

And I have an issue with one test running this locally but I can not explain why the test is failing. I get the following output. @bnoordhuis you wrote that test, would you be so kind and have a look at it? Because I do not see the connection of the error to my changes...

	async-hooks doctool inspector known_issues message parallel pseudo-tty sequential \
	addons addons-napi
[02:05|%  91|+ 1642|-   0]: release stack_overflow_async length differs.       
expect=4
actual=2
patterns:
pattern = ^RangeError\:\ Maximum\ call\ stack\ size\ exceeded$
pattern = ^\ \ \ \ at\ f\ \(.*test.*message.*stack\_overflow\_async\.js\:.*\)$
pattern = ^\ \ \ \ at\ f\ \(.*test.*message.*stack\_overflow\_async\.js\:7\:.*\)$
pattern = ^\ \ \ \ at\ f\ \(.*test.*message.*stack\_overflow\_async\.js\:7\:.*\)$
outlines:
outline = (node:21361) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): RangeError: Maximum call stack size exceeded
outline = (node:21361) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
=== release stack_overflow_async ===                    
Path: message/stack_overflow_async
(node:21361) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): RangeError: Maximum call stack size exceeded
(node:21361) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Command: out/Release/node --stack_trace_limit=3 /home/ruben/repos/node/node/test/message/stack_overflow_async.js
[02:46|% 100|+ 1785|-   1]: Done                                               

@addaleax
Copy link
Member

@BridgeAR That’s a really new test, and I’m pretty sure it’s not related to your PR

@BridgeAR
Copy link
Member Author

@BridgeAR
Copy link
Member Author

BridgeAR commented Aug 24, 2017

@addaleax the weird part is that is it happening only with this PR. On master it does not fail. It is reproducible for me on my local machine and also the CI fails exactly at this test. [EDIT] And the test passes if I call console.log() on top of the function call. Only calling console.log fails. [/EDIT]

@BridgeAR
Copy link
Member Author

@bnoordhuis it would be nice if you could have a look at this PR because it somehow breaks this test even though there should not be any connection. The only way for me to fix the test is by calling process.stdout.write("") (or console.log()) somewhere before the test starts.

@BridgeAR BridgeAR mentioned this pull request Aug 31, 2017
4 tasks
@BridgeAR
Copy link
Member Author

BridgeAR commented Sep 5, 2017

@nodejs/collaborators PTAL

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM with both CI and CITGM green. Those type of large scale perf improvements can lead to broken production code.

This should be semver-minor at least, and it should bake for LTS for some month before being backported.

addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 17, 2017
In addition use the newer common.expectsError version.

PR-URL: nodejs/node#14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@jasnell
Copy link
Member

jasnell commented Sep 20, 2017

This needs a backport pr to land in v8.x

jasnell pushed a commit that referenced this pull request Sep 20, 2017
PR-URL: #14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
jasnell pushed a commit that referenced this pull request Sep 20, 2017
In addition use the newer common.expectsError version.

PR-URL: #14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 20, 2017
PR-URL: nodejs#14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 20, 2017
The main optimizations are
- Removed visibleKeys
- Removed proxy cache
- Removed Object.assign
- No key concatenating anymore
- No key recalculating anymore
- Improved indentation logic
- Improved string escape logic
- Added many fast paths
- Optimized code branches a lot
- Optimized (boxed) primitive handling
- Inline code if possible
- Only check extra keys if necessary
- Guard against unnecessary more expensive calls

This also fixes a bug with special array number keys as e.g. "00".

Besides that there were lots of smaller optimizations, the
code got a bit cleaned up and a few more tests got in.

PR-URL: nodejs#14881
Fixes: nodejs#15288
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 20, 2017
This fixes a issue brought up in nodejs#15288.

PR-URL: nodejs#14881
Refs: nodejs#15288
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 20, 2017
Remove unnecessary code parts and outdated code

PR-URL: nodejs#14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
jasnell pushed a commit that referenced this pull request Sep 21, 2017
PR-URL: #14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
jasnell pushed a commit that referenced this pull request Sep 21, 2017
The main optimizations are
- Removed visibleKeys
- Removed proxy cache
- Removed Object.assign
- No key concatenating anymore
- No key recalculating anymore
- Improved indentation logic
- Improved string escape logic
- Added many fast paths
- Optimized code branches a lot
- Optimized (boxed) primitive handling
- Inline code if possible
- Only check extra keys if necessary
- Guard against unnecessary more expensive calls

This also fixes a bug with special array number keys as e.g. "00".

Besides that there were lots of smaller optimizations, the
code got a bit cleaned up and a few more tests got in.

PR-URL: #14881
Fixes: #15288
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
jasnell pushed a commit that referenced this pull request Sep 21, 2017
This fixes a issue brought up in #15288.

PR-URL: #14881
Refs: #15288
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
jasnell pushed a commit that referenced this pull request Sep 21, 2017
Remove unnecessary code parts and outdated code

PR-URL: #14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
PR-URL: nodejs/node#14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
PR-URL: nodejs/node#14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
The main optimizations are
- Removed visibleKeys
- Removed proxy cache
- Removed Object.assign
- No key concatenating anymore
- No key recalculating anymore
- Improved indentation logic
- Improved string escape logic
- Added many fast paths
- Optimized code branches a lot
- Optimized (boxed) primitive handling
- Inline code if possible
- Only check extra keys if necessary
- Guard against unnecessary more expensive calls

This also fixes a bug with special array number keys as e.g. "00".

Besides that there were lots of smaller optimizations, the
code got a bit cleaned up and a few more tests got in.

PR-URL: nodejs/node#14881
Fixes: nodejs/node#15288
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
This fixes a issue brought up in #15288.

PR-URL: nodejs/node#14881
Refs: nodejs/node#15288
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
Remove unnecessary code parts and outdated code

PR-URL: nodejs/node#14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
In addition use the newer common.expectsError version.

PR-URL: nodejs/node#14881
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@MylesBorins MylesBorins added baking-for-lts PRs that need to wait before landing in a LTS release. lts-watch-v6.x labels Oct 17, 2017
@MylesBorins
Copy link
Contributor

should this land in LTS? If so it will need to bake a bit longer.

Please change labels as appropriate

@MylesBorins MylesBorins removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Aug 17, 2018
@BridgeAR BridgeAR deleted the improve-inspect branch April 1, 2019 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Issues and PRs related to the benchmark subsystem. performance Issues and PRs related to the performance of Node.js. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.