build: allow enabling the --trace-maps flag in V8#14018
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nodejs-github-bot added the build 
refack commented Jul 1, 2017
Context please? |
node.gyp Outdated
configure Outdated
There was a problem hiding this comment.
Consider undocumenting this or add a (use at your own risk) because it introduces an incompatibility between builds with and without the flag.
It adds a field to SharedFunctionInfo that ends up in the code cache so the result of new vm.Script(source,{produceCachedData: true }) from a -DTRACE_MAPS build won't be loadable in a normal build and vice versa. I didn't test but I expect it segfaults.
There was a problem hiding this comment.
I'm +1 to documenting with a warning. Quite a few of configure's switches have wide reaching results...
bnoordhuis commented Jul 1, 2017
@refack It's a tool for tracing hidden class transitions. ('hidden class' == 'map' in V8 parlance.) |
evanlucas commented Jul 5, 2017
Updated, PTAL |
configure Outdated
There was a problem hiding this comment.
Actually, one nit: V8 with a capital V, since we started disambiguating due to Node (v)8 ;)
mhdawson commented Jul 6, 2017
This can be useful for tracing map creation. PR-URL: nodejs#14018 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
evanlucas commented Jul 7, 2017
Landed in 49d13a1. Thanks! |
addaleax commented Jul 11, 2017
@evanlucas This seems to conflict with the snapshot change, can you backport to v8.x? |
This can be useful for tracing map creation. PR-URL: nodejs#14018 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
This can be useful for tracing map creation. Backport-PR-URL: #14344 Backport-Reviewed-By: Ben Noordhuis <[email protected]> Backport-Reviewed-By: Refael Ackermann <[email protected]> PR-URL: #14018 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
This can be useful for tracing map creation. Backport-PR-URL: #14344 Backport-Reviewed-By: Ben Noordhuis <[email protected]> Backport-Reviewed-By: Refael Ackermann <[email protected]> PR-URL: #14018 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
MylesBorins commented Aug 16, 2017
Should this have been Should we backport to lts? |
MylesBorins commented Sep 19, 2017
ping @evanlucas |
evanlucas commented Oct 2, 2017
@MylesBorins sorry I somehow missed your previous comment. I think it could be argued either way that this is semver-minor. It isn't exposed in release builds and can only be done when using |
bnoordhuis commented Oct 24, 2017
https://chromium-review.googlesource.com/c/v8/v8/+/734648 -- |
MylesBorins commented Dec 19, 2017
I've landed on v6.x. Please lmk if there are any concerns |
This can be useful for tracing map creation. PR-URL: #14018 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
8 participants
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
build
This can be useful for tracing map creation.