★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/24394Nouvelle recherche | Portail wanayoo
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

Jump to bottom

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

Merged
merged 1 commit into from Nov 17, 2018
Merged

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

merged 1 commit into from Nov 17, 2018

Conversation

refack
Copy link
Member

@refack refack commented Nov 16, 2018 •
edited

This reverts commit e2a8e32.
This reverts commit 715bbb9.

Fixes: #24393

Please 👍 to fast-track

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

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Nov 16, 2018

@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 refack requested review from hashseed and psmarshall Nov 16, 2018
@refack
Copy link
Member Author

@refack refack commented Nov 16, 2018 •
edited

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 refack requested a review from targos Nov 16, 2018
@refack refack mentioned this pull request Nov 16, 2018
Closed
@refack
Copy link
Member Author

@refack refack commented Nov 16, 2018

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.

@refack refack requested a review from Trott Nov 16, 2018
Trott
Trott approved these changes Nov 16, 2018
View changes
Copy link
Member

@Trott Trott left a comment

Rubber stamp LGTM.

targos
targos approved these changes Nov 17, 2018
View changes
@targos
Copy link
Member

@targos targos commented Nov 17, 2018

@refack I believe this can land.

@refack
Revert "src: enable detailed source positions in V8"
6adbe9a 
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 refack force-pushed the revert-v8-source-possition branch from d237f16 to 6adbe9a Nov 17, 2018
@refack
Copy link
Member Author

@refack refack commented Nov 17, 2018

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

@refack refack merged commit 6adbe9a into nodejs:master Nov 17, 2018
1 of 2 checks passed
@refack refack deleted the revert-v8-source-possition branch Nov 17, 2018
@targos targos mentioned this pull request Nov 17, 2018
Closed
2 tasks
@refack
Copy link
Member Author

@refack refack commented Nov 17, 2018 •
edited

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)

targos added a commit that referenced this issue Nov 18, 2018
@refack @targos
Revert "src: enable detailed source positions in V8"
d92bd03 
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>
@hashseed hashseed mentioned this pull request Nov 20, 2018
Closed
3 tasks
addaleax added a commit that referenced this issue Nov 23, 2018
@hashseed @addaleax
deps: cherry-pick 073073b from upstream V8
39db8b1 
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>
addaleax added a commit that referenced this issue Nov 23, 2018
@hashseed @addaleax
src: enable detailed source positions in V8
12b69f9 
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>
targos added a commit that referenced this issue Nov 24, 2018
@hashseed @targos
deps: cherry-pick 073073b from upstream V8
0ae44bb 
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>
targos added a commit that referenced this issue Nov 24, 2018
@hashseed @targos
src: enable detailed source positions in V8
85c1d62 
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>
rvagg added a commit that referenced this issue Nov 28, 2018
@refack @rvagg
Revert "src: enable detailed source positions in V8"
a7f6c04 
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>
rvagg added a commit that referenced this issue Nov 28, 2018
@hashseed @rvagg
deps: cherry-pick 073073b from upstream V8
61179e6 
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>
rvagg added a commit that referenced this issue Nov 28, 2018
@hashseed @rvagg
src: enable detailed source positions in V8
aa220cf 
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>
targos added a commit to targos/node that referenced this issue Dec 4, 2018
@hashseed @targos
deps: cherry-pick 073073b from upstream V8
cbce4e4 
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>
@BridgeAR BridgeAR mentioned this pull request Dec 5, 2018
Merged
4 tasks
pull bot pushed a commit to shakir-abdo/node that referenced this issue Dec 6, 2018
@hashseed @targos
deps: cherry-pick 073073b from upstream V8
d088007 
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>
@beevelop beevelop mentioned this pull request Dec 7, 2018
Closed
@beevelop beevelop mentioned this pull request Dec 7, 2018
Closed
@beevelop beevelop mentioned this pull request Dec 7, 2018
Closed
@beevelop beevelop mentioned this pull request Dec 8, 2018
Closed
@beevelop beevelop mentioned this pull request Dec 8, 2018
Closed
refack added a commit to refack/node that referenced this issue Jan 14, 2019
@hashseed @refack
deps: cherry-pick 073073b from upstream V8
41f20f0 
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>
refack added a commit to refack/node that referenced this issue Jan 14, 2019
@hashseed @refack
src: enable detailed source positions in V8
c0e6c6a 
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>
refack added a commit to refack/node that referenced this issue Jan 14, 2019
@hashseed @refack
deps: cherry-pick 073073b from upstream V8
7e0ce22 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott

@targos targos

@joyeecheung joyeecheung

@ryzokuken ryzokuken

@hashseed hashseed

@psmarshall psmarshall

Assignees
No one assigned
Labels
fast-track lib / src regression v8 engine
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

V8: test-v8 is broken
6 participants
@refack @nodejs-github-bot @targos @Trott @joyeecheung @ryzokuken