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.

Sign up for GitHub

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

deps: V8: backport 14ac02c from upstream #17512

Closed
wants to merge 1 commit into from
Closed

deps: V8: backport 14ac02c from upstream #17512

wants to merge 1 commit into from

Conversation

ofrobots
Copy link
Contributor

@ofrobots ofrobots commented Dec 7, 2017 •
edited
Loading

This fixes a memory leak in the CPU profiler: https://bugs.chromium.org/p/v8/issues/detail?id=6623

This affects master, 9.x and 8.x. master should pick this up through a regular V8 update (if the merge gets approved). This PR is for 9.x. This should cherry-pick to 8.x easily except for the version bump, which will need to be migrated to v8-version.h.

Original commit message:

[cpu-profiler] Clear code entries when no observers are present.

Performed manual testing as well by making 20 CPU profile recordings of
loading http://meduza.io page. Without the patch the page renderer memory size
grows beyond 300MB. With the patch it remains below 200MB.

BUG=v8:6623

Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
Reviewed-on: https://chromium-review.googlesource.com/798020
Commit-Queue: Alexei Filippov [email protected]
Reviewed-by: Yang Guo [email protected]
Cr-Commit-Position: refs/heads/master@{#49914}

Ref: v8/v8@14ac02c

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

deps:V8

R = @nodejs/v8
CI: https://ci.nodejs.org/job/node-test-pull-request/11929/
V8-CI: https://ci.nodejs.org/view/All/job/node-test-commit-v8-linux/1102/

@ofrobots
deps: V8: backport 14ac02c from upstream
f4dbe8a 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{nodejs#49914}

Ref: v8/v8@14ac02c
@ofrobots ofrobots added dont-land-on-v6.x v8 engine Issues and PRs related to the V8 dependency. labels Dec 7, 2017
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. v9.x labels Dec 7, 2017
TimothyGu
TimothyGu approved these changes Dec 8, 2017
View reviewed changes
Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

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

Rubberstamp LGTM.

@addaleax addaleax removed the v8.x label Dec 11, 2017
mhdawson
mhdawson approved these changes Dec 11, 2017
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

Rubber Stamp LGTM

@ofrobots
Copy link
Contributor Author

ofrobots commented Dec 12, 2017 •
edited
Loading

Looking at the V8 CI, the same test is failing in all four environments. I reran the V8-CI against v9.x: https://ci.nodejs.org/job/node-test-commit-v8-linux/1109/. The same test is failing there as well. Running the V8 tests locally against the V8 branch-heads/6.2 branch with and without the patch passes the tests. This must be an issue in the V8-CI or a regression in a floating patch. /cc @mhdawson any ideas?

I think this PR is good to go for 8.x and 9.x. /cc @gibfahn

MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@ofrobots @MylesBorins
deps: V8: backport 14ac02c from upstream
2f919ed 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#49914}

Ref: v8/v8@14ac02c
PR-URL: #17512
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@MylesBorins
Copy link
Contributor

Landed in 2f919ed

Failing test was fixed in #17383
fips failure is unrelated (fix for flake also landed)

MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@ofrobots @MylesBorins
deps: V8: backport 14ac02c from upstream
5e73c1f 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#49914}

Ref: v8/v8@14ac02c
PR-URL: #17512
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@ofrobots @MylesBorins
deps: V8: backport 14ac02c from upstream
e2a5633 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#49914}

Ref: v8/v8@14ac02c
PR-URL: #17512
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@ofrobots @MylesBorins
deps: V8: backport 14ac02c from upstream
49d23a3 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#49914}

Ref: v8/v8@14ac02c
PR-URL: #17512
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
@gibfahn
Copy link
Member

gibfahn commented Dec 13, 2017 •
edited
Loading

@MylesBorins I'm getting an error with this on v8.x-staging.

I'm going to back it out for now, but if I'm doing something wrong let me know and we can re-land.

CI to check: https://ci.nodejs.org/job/node-test-commit/14803/

▶▶▶ ./configure && make -j4                                ~/wrk/com/DANGER/node 7s (v8.x-staging)
creating ./icu_config.gypi
* Using ICU in deps/icu-small
creating ./icu_config.gypi
{ 'target_defaults': { 'cflags': [],
                       'default_configuration': 'Release',
                       'defines': [],
                       'include_dirs': [],
                       'libraries': []},
  'variables': { 'asan': 0,
                 'coverage': 'false',
                 'debug_devtools': 'node',
                 'debug_http2': 'false',
                 'debug_nghttp2': 'false',
                 'force_dynamic_crt': 0,
                 'host_arch': 'x64',
                 'icu_data_file': 'icudt59l.dat',
                 'icu_data_in': '../../deps/icu-small/source/data/in/icudt59l.dat',
                 'icu_endianness': 'l',
                 'icu_gyp_path': 'tools/icu/icu-generic.gyp',
                 'icu_locales': 'en,root',
                 'icu_path': 'deps/icu-small',
                 'icu_small': 'true',
                 'icu_ver_major': '59',
                 'llvm_version': 0,
                 'node_byteorder': 'little',
                 'node_enable_d8': 'false',
                 'node_enable_v8_vtunejit': 'false',
                 'node_install_npm': 'true',
                 'node_module_version': 57,
                 'node_no_browser_globals': 'false',
                 'node_prefix': '/usr/local',
                 'node_release_urlbase': '',
                 'node_shared': 'false',
                 'node_shared_cares': 'false',
                 'node_shared_http_parser': 'false',
                 'node_shared_libuv': 'false',
                 'node_shared_openssl': 'false',
                 'node_shared_zlib': 'false',
                 'node_tag': '',
                 'node_use_bundled_v8': 'true',
                 'node_use_dtrace': 'true',
                 'node_use_etw': 'false',
                 'node_use_lttng': 'false',
                 'node_use_openssl': 'true',
                 'node_use_perfctr': 'false',
                 'node_use_v8_platform': 'true',
                 'node_without_node_options': 'false',
                 'openssl_fips': '',
                 'openssl_no_asm': 0,
                 'shlib_suffix': '57.dylib',
                 'target_arch': 'x64',
                 'uv_parent_path': '/deps/uv/',
                 'uv_use_dtrace': 'true',
                 'v8_enable_gdbjit': 0,
                 'v8_enable_i18n_support': 1,
                 'v8_enable_inspector': 1,
                 'v8_no_strict_aliasing': 1,
                 'v8_optimized_debug': 0,
                 'v8_promise_internal_field_count': 1,
                 'v8_random_seed': 0,
                 'v8_trace_maps': 0,
                 'v8_use_snapshot': 'true',
                 'want_separate_host_toolset': 0,
                 'xcode_version': '9.0'}}
creating ./config.gypi
creating ./config.mk
xcode-select: error: tool 'xcodebuild' requires Xcode, but active developer directory '/Library/Developer/CommandLineTools' is a command line tools instance

xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'

/Library/Developer/CommandLineTools/usr/bin/make -C out BUILDTYPE=Release V=1
  LD_LIBRARY_PATH=/Users/gib/wrk/com/DANGER/node/out/Release/lib.host:/Users/gib/wrk/com/DANGER/node/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../.; mkdir -p /Users/gib/wrk/com/DANGER/node/out/Release/obj/gen; python tools/js2c.py "/Users/gib/wrk/com/DANGER/node/out/Release/obj/gen/node_javascript.cc" lib/internal/bootstrap_node.js lib/async_hooks.js lib/assert.js lib/buffer.js lib/child_process.js lib/console.js lib/constants.js lib/crypto.js lib/cluster.js lib/dgram.js lib/dns.js lib/domain.js lib/events.js lib/fs.js lib/http.js lib/http2.js lib/_http_agent.js lib/_http_client.js lib/_http_common.js lib/_http_incoming.js lib/_http_outgoing.js lib/_http_server.js lib/https.js lib/inspector.js lib/module.js lib/net.js lib/os.js lib/path.js lib/perf_hooks.js lib/process.js lib/punycode.js lib/querystring.js lib/readline.js lib/repl.js lib/stream.js lib/_stream_readable.js lib/_stream_writable.js lib/_stream_duplex.js lib/_stream_transform.js lib/_stream_passthrough.js lib/_stream_wrap.js lib/string_decoder.js lib/sys.js lib/timers.js lib/tls.js lib/_tls_common.js lib/_tls_legacy.js lib/_tls_wrap.js lib/tty.js lib/url.js lib/util.js lib/v8.js lib/vm.js lib/zlib.js lib/internal/buffer.js lib/internal/child_process.js lib/internal/cluster/child.js lib/internal/cluster/master.js lib/internal/cluster/round_robin_handle.js lib/internal/cluster/shared_handle.js lib/internal/cluster/utils.js lib/internal/cluster/worker.js lib/internal/encoding.js lib/internal/errors.js lib/internal/freelist.js lib/internal/fs.js lib/internal/http.js lib/internal/inspector_async_hook.js lib/internal/linkedlist.js lib/internal/loader/Loader.js lib/internal/loader/ModuleMap.js lib/internal/loader/ModuleJob.js lib/internal/loader/ModuleWrap.js lib/internal/loader/ModuleRequest.js lib/internal/loader/search.js lib/internal/safe_globals.js lib/internal/net.js lib/internal/module.js lib/internal/os.js lib/internal/process/next_tick.js lib/internal/process/promises.js lib/internal/process/stdio.js lib/internal/process/warning.js lib/internal/process.js lib/internal/querystring.js lib/internal/process/write-coverage.js lib/internal/readline.js lib/internal/repl.js lib/internal/socket_list.js lib/internal/test/unicode.js lib/internal/url.js lib/internal/util.js lib/internal/util/types.js lib/internal/http2/core.js lib/internal/http2/compat.js lib/internal/http2/util.js lib/internal/v8_prof_polyfill.js lib/internal/v8_prof_processor.js lib/internal/streams/lazy_transform.js lib/internal/streams/BufferList.js lib/internal/streams/legacy.js lib/internal/streams/destroy.js lib/internal/wrap_js_stream.js deps/v8/tools/splaytree.js deps/v8/tools/codemap.js deps/v8/tools/consarray.js deps/v8/tools/csvparser.js deps/v8/tools/profile.js deps/v8/tools/profile_view.js deps/v8/tools/logreader.js deps/v8/tools/tickprocessor.js deps/v8/tools/SourceMap.js deps/v8/tools/tickprocessor-driver.js deps/node-inspect/lib/_inspect.js deps/node-inspect/lib/internal/inspect_client.js deps/node-inspect/lib/internal/inspect_repl.js ./config.gypi src/nolttng_macros.py src/noperfctr_macros.py
  touch d42b6d90f6c5956ee6690859e54e930d3e9df0b6.intermediate
  LD_LIBRARY_PATH=/Users/gib/wrk/com/DANGER/node/out/Release/lib.host:/Users/gib/wrk/com/DANGER/node/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../deps/v8/src/inspector; mkdir -p /Users/gib/wrk/com/DANGER/node/out/Release/obj/gen/src/inspector/protocol /Users/gib/wrk/com/DANGER/node/out/Release/obj/gen/include/inspector; python ../../third_party/inspector_protocol/CodeGenerator.py --jinja_dir ../../third_party --output_base "/Users/gib/wrk/com/DANGER/node/out/Release/obj/gen/src/inspector" --config inspector_protocol_config.json
  c++ '-DV8_GYP_BUILD' '-D_DARWIN_USE_64_BIT_INODE=1' '-DV8_TARGET_ARCH_X64' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT' '-Dv8_promise_internal_field_count' '-DV8_INTL_SUPPORT' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' -I../deps/v8 -I../. -I/Users/gib/wrk/com/DANGER/node/out/Release/obj/gen -I../deps/v8/include -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common  -O3 -gdwarf-2 -fstrict-aliasing -mmacosx-version-min=10.7 -arch x86_64 -Wall -Wendif-labels -W -Wno-unused-parameter -std=gnu++0x -stdlib=libc++ -fno-rtti -fno-exceptions -fno-threadsafe-statics -fno-strict-aliasing -MMD -MF /Users/gib/wrk/com/DANGER/node/out/Release/.deps//Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/profiler/profiler-listener.o.d.raw   -c -o /Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/profiler/profiler-listener.o ../deps/v8/src/profiler/profiler-listener.cc
  c++ '-DV8_GYP_BUILD' '-D_DARWIN_USE_64_BIT_INODE=1' '-DV8_TARGET_ARCH_X64' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT' '-Dv8_promise_internal_field_count' '-DV8_INTL_SUPPORT' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' -I../deps/v8 -I../. -I/Users/gib/wrk/com/DANGER/node/out/Release/obj/gen -I../deps/v8/include -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common  -O3 -gdwarf-2 -fstrict-aliasing -mmacosx-version-min=10.7 -arch x86_64 -Wall -Wendif-labels -W -Wno-unused-parameter -std=gnu++0x -stdlib=libc++ -fno-rtti -fno-exceptions -fno-threadsafe-statics -fno-strict-aliasing -MMD -MF /Users/gib/wrk/com/DANGER/node/out/Release/.deps//Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/profiler/strings-storage.o.d.raw   -c -o /Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/profiler/strings-storage.o ../deps/v8/src/profiler/strings-storage.cc
  c++ '-DV8_GYP_BUILD' '-D_DARWIN_USE_64_BIT_INODE=1' '-DV8_TARGET_ARCH_X64' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT' '-Dv8_promise_internal_field_count' '-DV8_INTL_SUPPORT' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' -I../deps/v8 -I../. -I/Users/gib/wrk/com/DANGER/node/out/Release/obj/gen -I../deps/v8/include -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common  -O3 -gdwarf-2 -fstrict-aliasing -mmacosx-version-min=10.7 -arch x86_64 -Wall -Wendif-labels -W -Wno-unused-parameter -std=gnu++0x -stdlib=libc++ -fno-rtti -fno-exceptions -fno-threadsafe-statics -fno-strict-aliasing -MMD -MF /Users/gib/wrk/com/DANGER/node/out/Release/.deps//Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/profiler/tick-sample.o.d.raw   -c -o /Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/profiler/tick-sample.o ../deps/v8/src/profiler/tick-sample.cc
  c++ '-DV8_GYP_BUILD' '-D_DARWIN_USE_64_BIT_INODE=1' '-DV8_TARGET_ARCH_X64' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT' '-Dv8_promise_internal_field_count' '-DV8_INTL_SUPPORT' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' -I../deps/v8 -I../. -I/Users/gib/wrk/com/DANGER/node/out/Release/obj/gen -I../deps/v8/include -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common  -O3 -gdwarf-2 -fstrict-aliasing -mmacosx-version-min=10.7 -arch x86_64 -Wall -Wendif-labels -W -Wno-unused-parameter -std=gnu++0x -stdlib=libc++ -fno-rtti -fno-exceptions -fno-threadsafe-statics -fno-strict-aliasing -MMD -MF /Users/gib/wrk/com/DANGER/node/out/Release/.deps//Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/profiler/tracing-cpu-profiler.o.d.raw   -c -o /Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/profiler/tracing-cpu-profiler.o ../deps/v8/src/profiler/tracing-cpu-profiler.cc
  c++ '-DV8_GYP_BUILD' '-D_DARWIN_USE_64_BIT_INODE=1' '-DV8_TARGET_ARCH_X64' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT' '-Dv8_promise_internal_field_count' '-DV8_INTL_SUPPORT' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' '-DUCONFIG_NO_SERVICE=1' '-DUCONFIG_NO_REGULAR_EXPRESSIONS=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=0' '-DUCONFIG_NO_BREAK_ITERATION=0' -I../deps/v8 -I../. -I/Users/gib/wrk/com/DANGER/node/out/Release/obj/gen -I../deps/v8/include -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common  -O3 -gdwarf-2 -fstrict-aliasing -mmacosx-version-min=10.7 -arch x86_64 -Wall -Wendif-labels -W -Wno-unused-parameter -std=gnu++0x -stdlib=libc++ -fno-rtti -fno-exceptions -fno-threadsafe-statics -fno-strict-aliasing -MMD -MF /Users/gib/wrk/com/DANGER/node/out/Release/.deps//Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/property-descriptor.o.d.raw   -c -o /Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/property-descriptor.o ../deps/v8/src/property-descriptor.cc
../deps/v8/src/profiler/profiler-listener.cc:285:49: error: no member named 'make_unique' in namespace 'v8::base'
  std::unique_ptr<CodeEntry> code_entry = base::make_unique<CodeEntry>(
                                          ~~~~~~^
../deps/v8/src/profiler/profiler-listener.cc:285:61: error: 'CodeEntry' does not refer to a value
  std::unique_ptr<CodeEntry> code_entry = base::make_unique<CodeEntry>(
                                                            ^
../deps/v8/src/profiler/cpu-profiler.h:25:7: note: declared here
class CodeEntry;
      ^
../deps/v8/src/profiler/profiler-listener.cc:286:7: warning: expression result unused [-Wunused-value]
      tag, name, name_prefix, resource_name, line_number, column_number,
      ^~~
../deps/v8/src/profiler/profiler-listener.cc:286:12: warning: expression result unused [-Wunused-value]
      tag, name, name_prefix, resource_name, line_number, column_number,
           ^~~~
../deps/v8/src/profiler/profiler-listener.cc:286:18: warning: expression result unused [-Wunused-value]
      tag, name, name_prefix, resource_name, line_number, column_number,
                 ^~~~~~~~~~~
../deps/v8/src/profiler/profiler-listener.cc:286:31: warning: expression result unused [-Wunused-value]
      tag, name, name_prefix, resource_name, line_number, column_number,
                              ^~~~~~~~~~~~~
../deps/v8/src/profiler/profiler-listener.cc:286:46: warning: expression result unused [-Wunused-value]
      tag, name, name_prefix, resource_name, line_number, column_number,
                                             ^~~~~~~~~~~
../deps/v8/src/profiler/profiler-listener.cc:286:59: warning: expression result unused [-Wunused-value]
      tag, name, name_prefix, resource_name, line_number, column_number,
                                                          ^~~~~~~~~~~~~
../deps/v8/src/profiler/profiler-listener.cc:287:7: warning: expression result unused [-Wunused-value]
      line_info, instruction_start);
      ^~~~~~~~~
7 warnings and 2 errors generated.
make[1]: *** [/Users/gib/wrk/com/DANGER/node/out/Release/obj.target/v8_base/deps/v8/src/profiler/profiler-listener.o] Error 1
make[1]: *** Waiting for unfinished jobs....
rm d42b6d90f6c5956ee6690859e54e930d3e9df0b6.intermediate
make: *** [node] Error 2

@gibfahn gibfahn reopened this Dec 13, 2017
@ofrobots
Copy link
Contributor Author

@gibfahn I can reproduce the issue. I am investigating.

ofrobots added a commit to ofrobots/node that referenced this pull request Dec 13, 2017
@ofrobots
deps: V8: backport 14ac02c from upstream
59c3996 
This is the V8 6.1 specific backport for Node 8.x.

Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{nodejs#49914}

Ref: v8/v8@14ac02c
Ref: nodejs#17512
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@ofrobots ofrobots mentioned this pull request Dec 13, 2017
Closed
2 tasks
@ofrobots
Copy link
Contributor Author

Additional fixup was needed for 8.x (V8 6.1). I have opened a separate 8.x specific PR at #17659.

Attaching dont-land-on-8.x label here.

@ofrobots ofrobots closed this Dec 13, 2017
@ofrobots ofrobots deleted the v8-6623 branch December 13, 2017 18:36
@TimothyGu TimothyGu mentioned this pull request Dec 14, 2017
Closed
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
@ofrobots @gibfahn
deps: V8: backport 14ac02c from upstream
f34ee5c 
This is the V8 6.1 specific backport for Node 8.x.

Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#49914}

Ref: v8/v8@14ac02c
PR-URL: #17512
Backport-PR-URL: #17659
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
This was referenced Dec 20, 2017
Closed
Merged
targos pushed a commit to targos/node that referenced this pull request Jan 15, 2018
@ofrobots @targos
deps: V8: backport 14ac02c from upstream
1d55efe 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{nodejs#49914}

Ref: v8/v8@14ac02c
PR-URL: nodejs#17512
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 16, 2018
@ofrobots @gibfahn
deps: V8: backport 14ac02c from upstream
be02cb9 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{nodejs#49914}

Ref: v8/v8@14ac02c
PR-URL: nodejs#17512
Backport-PR-URL: nodejs#16413
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 22, 2018
@ofrobots @gibfahn
deps: V8: backport 14ac02c from upstream
8b757a8 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{nodejs#49914}

Ref: v8/v8@14ac02c
PR-URL: nodejs#17512
Backport-PR-URL: nodejs#16413
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit to targos/node that referenced this pull request Feb 7, 2018
@ofrobots @MylesBorins
deps: V8: backport 14ac02c from upstream
a10b18e 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{nodejs#49914}

Ref: v8/v8@14ac02c
PR-URL: nodejs#17512
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gibfahn pushed a commit that referenced this pull request Feb 18, 2018
@ofrobots @gibfahn
deps: V8: backport 14ac02c from upstream
51ad36a 
Original commit message:
  [cpu-profiler] Clear code entries when no observers are present.

  Performed manual testing as well by making 20 CPU profile recordings of
  loading http://meduza.io page. Without the patch the page renderer memory size
  grows beyond 300MB. With the patch it remains below 200MB.

  BUG=v8:6623

  Change-Id: Ifce541b84bb2aaaa5175520f8dd49dbc0cb5dd20
  Reviewed-on: https://chromium-review.googlesource.com/798020
  Commit-Queue: Alexei Filippov <[email protected]>
  Reviewed-by: Yang Guo <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#49914}

Ref: v8/v8@14ac02c
PR-URL: #17512
Backport-PR-URL: #16413
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

@targos targos targos approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ofrobots @MylesBorins @gibfahn @TimothyGu @targos @mhdawson @addaleax @nodejs-github-bot