Revert "src: enable detailed source positions in V8" #24394

refack · 2018-11-16T14:45:32Z

This reverts commit e2a8e32.
This reverts commit 715bbb9.

Please 👍 to fast-track

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

nodejs-github-bot · 2018-11-16T14:45:34Z

@refack build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1634/pipeline

refack · 2018-11-16T14:46:52Z

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1860/ ✔️
~~PR CI: https://ci.nodejs.org/job/node-test-pull-request/18674/~~ (flake #24397)
~~Resume: https://ci.nodejs.org/job/node-test-pull-request/18676/~~ (flake #24403)
Resume CI: https://ci.nodejs.org/job/node-test-pull-request/18689/ ✔️

refack · 2018-11-16T14:48:12Z

From #24393 (comment)

@targos:

Error in test compilation:

15:28:41 FAILED: obj/test/cctest/cctest_sources/test-cpu-profiler.o 
15:28:41 ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/test/cctest/cctest_sources/test-cpu-profiler.o.d -DV8_INTL_SUPPORT -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_GNU_SOURCE -DCR_CLANG_REVISION=\"338452-1\" -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS -DCR_LIBCXX_REVISION=332543 -DCR_LIBCXXABI_REVISION=331450 -DCR_SYSROOT_HASH=815a8c22f8657fe57d02e2c2d893bcdc25a243cf -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64 -DENABLE_GDB_JIT_INTERFACE -DENABLE_MINOR_MC -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -DV8_INTL_SUPPORT -DV8_USE_SNAPSHOT -DV8_USE_EXTERNAL_STARTUP_DATA -DV8_CONCURRENT_MARKING -DV8_EMBEDDED_BUILTINS -DV8_TARGET_ARCH_X64 -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -I../.. -Igen -I../../include -Igen/include -I../.. -Igen -I../../include -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -fPIC -B../../third_party/binutils/Linux_x64/Release/bin -pthread -fcolor-diagnostics -fmerge-all-constants -Xclang -mllvm -Xclang -instcombine-lower-dbg-declare=0 -no-canonical-prefixes -fcomplete-member-pointers -m64 -march=x86-64 -Wall -Werror -Wextra -Wimplicit-fallthrough -Wthread-safety -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-user-defined-warnings -Wno-unused-lambda-capture -Wno-null-pointer-arithmetic -Wno-enum-compare-switch -Wno-ignored-pragma-optimize -fno-omit-frame-pointer -g0 -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wmissing-field-initializers -Winconsistent-missing-override -Wunreachable-code -Wshorten-64-to-32 -O3 -fno-ident -fdata-sections -ffunction-sections -std=c++14 -fno-exceptions -fno-rtti -nostdinc++ -isystem../../buildtools/third_party/libc++/trunk/include -isystem../../buildtools/third_party/libc++abi/trunk/include --sysroot=../../build/linux/debian_sid_amd64-sysroot -fvisibility-inlines-hidden -c ../../test/cctest/test-cpu-profiler.cc -o obj/test/cctest/cctest_sources/test-cpu-profiler.o
15:28:41 ../../test/cctest/test-cpu-profiler.cc:2574:34: error: use of undeclared identifier 'GetSourcePositionEntryCount'
15:28:41     int non_detailed_positions = GetSourcePositionEntryCount(i_isolate, source);
15:28:41                                  ^
15:28:41 ../../test/cctest/test-cpu-profiler.cc:2579:30: error: use of undeclared identifier 'GetSourcePositionEntryCount'
15:28:41     int detailed_positions = GetSourcePositionEntryCount(i_isolate, source);
15:28:41                              ^
15:28:41 2 errors generated.

Trott

Rubber stamp LGTM.

targos · 2018-11-17T14:17:30Z

@refack I believe this can land.

This reverts commit e2a8e32. This reverts commit 715bbb9. PR-URL: nodejs#24394 Fixes: nodejs#24393 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

refack · 2018-11-17T16:34:19Z

Extra sanity test V8 port land: https://ci.nodejs.org/job/node-test-commit-v8-linux/1866/

refack · 2018-11-17T17:01:17Z

Post land CI shows a failed test:
mjsunit_wasm_jsapi_harness on ppcle-ubuntu1404

stdout:
/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/test/mjsunit/wasm/jsapi-harness.js:102: Error loading file
load("test/wasm-js/test/harness/wasm-constants.js");
^
Command: /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/out.gn/ppc64.release/d8 --test /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/test/mjsunit/mjsunit.js /home/iojs/build/workspace/node-test-commit-v8-linux/nodes/ppcle-ubuntu1404/v8test/v8test/deps/v8/test/mjsunit/wasm/jsapi-harness.js --random-seed=-50650634 --nohard-abort --expose-wasm --allow-natives-syntax

Rerunning: https://ci.nodejs.org/job/node-test-commit-v8-linux/1867/ ❌ (fail repetead in job 1867)

This reverts commit e2a8e32. This reverts commit 715bbb9. PR-URL: #24394 Fixes: #24393 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Original commit message: [profiler] introduce API to enable detailed source positions This allows Node.js to enable detailed source positions for optimized code early on, without having to pass a flag string. R=petermarshall@chromium.org Change-Id: Ie74ea41f600cf6e31acbe802116df4976ccf1c75 Reviewed-on: https://chromium-review.googlesource.com/c/1319757 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#57380} Refs: v8/v8@073073b PR-URL: #24515 Refs: #24274 Refs: #24394 Refs: #24393 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Peter Marshall <petermarshall@chromium.org> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: #24515 Refs: #24274 Refs: #24394 Refs: #24393 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Peter Marshall <petermarshall@chromium.org> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

Original commit message: [profiler] introduce API to enable detailed source positions This allows Node.js to enable detailed source positions for optimized code early on, without having to pass a flag string. R=petermarshall@chromium.org Change-Id: Ie74ea41f600cf6e31acbe802116df4976ccf1c75 Reviewed-on: https://chromium-review.googlesource.com/c/1319757 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#57380} Refs: v8/v8@073073b PR-URL: #24515 Refs: #24274 Refs: #24394 Refs: #24393 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Peter Marshall <petermarshall@chromium.org> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: #24515 Refs: #24274 Refs: #24394 Refs: #24393 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Peter Marshall <petermarshall@chromium.org> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

This reverts commit e2a8e32. This reverts commit 715bbb9. PR-URL: #24394 Fixes: #24393 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Original commit message: [profiler] introduce API to enable detailed source positions This allows Node.js to enable detailed source positions for optimized code early on, without having to pass a flag string. R=petermarshall@chromium.org Change-Id: Ie74ea41f600cf6e31acbe802116df4976ccf1c75 Reviewed-on: https://chromium-review.googlesource.com/c/1319757 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#57380} Refs: v8/v8@073073b PR-URL: #24515 Refs: #24274 Refs: #24394 Refs: #24393 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Peter Marshall <petermarshall@chromium.org> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: #24515 Refs: #24274 Refs: #24394 Refs: #24393 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Peter Marshall <petermarshall@chromium.org> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

Original commit message: [profiler] introduce API to enable detailed source positions This allows Node.js to enable detailed source positions for optimized code early on, without having to pass a flag string. R=petermarshall@chromium.org Change-Id: Ie74ea41f600cf6e31acbe802116df4976ccf1c75 Reviewed-on: https://chromium-review.googlesource.com/c/1319757 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#57380} Refs: v8/v8@073073b PR-URL: nodejs#24515 Refs: nodejs#24274 Refs: nodejs#24394 Refs: nodejs#24393 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Peter Marshall <petermarshall@chromium.org> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: nodejs#24515 Refs: nodejs#24274 Refs: nodejs#24394 Refs: nodejs#24393 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Peter Marshall <petermarshall@chromium.org> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

Original commit message: [profiler] introduce API to enable detailed source positions This allows Node.js to enable detailed source positions for optimized code early on, without having to pass a flag string. R=petermarshall@chromium.org Change-Id: Ie74ea41f600cf6e31acbe802116df4976ccf1c75 Reviewed-on: https://chromium-review.googlesource.com/c/1319757 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#57380} Refs: v8/v8@073073b PR-URL: nodejs#24515 Refs: nodejs#24274 Refs: nodejs#24394 Refs: nodejs#24393 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Peter Marshall <petermarshall@chromium.org> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Nov 16, 2018

refack requested review from hashseed and psmarshall November 16, 2018 14:46

refack added v8 engine Issues and PRs related to the V8 dependency. regression Issues related to regressions. labels Nov 16, 2018

refack requested a review from targos November 16, 2018 14:46

refack mentioned this pull request Nov 16, 2018

V8: test-v8 is broken #24393

Closed

refack added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 16, 2018

refack requested a review from Trott November 16, 2018 14:50

Trott approved these changes Nov 16, 2018

View reviewed changes

joyeecheung approved these changes Nov 17, 2018

View reviewed changes

ryzokuken approved these changes Nov 17, 2018

View reviewed changes

targos approved these changes Nov 17, 2018

View reviewed changes

targos added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 17, 2018

refack force-pushed the revert-v8-source-possition branch from d237f16 to 6adbe9a Compare November 17, 2018 16:32

refack removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 17, 2018

refack merged commit 6adbe9a into nodejs:master Nov 17, 2018

refack deleted the revert-v8-source-possition branch November 17, 2018 16:34

targos added dont-land-on-v6.x labels Nov 17, 2018

targos mentioned this pull request Nov 17, 2018

src: enable detailed source positions in V8 #24274

Closed

2 tasks

hashseed mentioned this pull request Nov 20, 2018

src: enable detailed source positions in V8 #24515

Closed

3 tasks

BridgeAR mentioned this pull request Dec 5, 2018

v11.4.0 proposal #24854

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "src: enable detailed source positions in V8" #24394

Revert "src: enable detailed source positions in V8" #24394

refack commented Nov 16, 2018 •

edited

nodejs-github-bot commented Nov 16, 2018

refack commented Nov 16, 2018 •

edited

refack commented Nov 16, 2018

Trott left a comment

targos commented Nov 17, 2018

refack commented Nov 17, 2018

refack commented Nov 17, 2018 •

edited

Revert "src: enable detailed source positions in V8" #24394

Revert "src: enable detailed source positions in V8" #24394

Conversation

refack commented Nov 16, 2018 • edited

Checklist

nodejs-github-bot commented Nov 16, 2018

refack commented Nov 16, 2018 • edited

refack commented Nov 16, 2018

Trott left a comment

Choose a reason for hiding this comment

targos commented Nov 17, 2018

refack commented Nov 17, 2018

refack commented Nov 17, 2018 • edited

refack commented Nov 16, 2018 •

edited

refack commented Nov 16, 2018 •

edited

refack commented Nov 17, 2018 •

edited