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

Tests on Windows take forever to start #179

Closed
seishun opened this issue Dec 17, 2014 · 10 comments
Closed

Tests on Windows take forever to start #179

seishun opened this issue Dec 17, 2014 · 10 comments
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.

Comments

@seishun
Copy link
Contributor

seishun commented Dec 17, 2014

For some reason, whenever I run vcbuild test nosign, msbuild decides to rebuild node_javascript.cc and subsequently re-link node.lib, which for some reason takes several minutes on a Core i7 machine (many times longer than in Visual Studio). This behavior has been observed on two different machines.

I'm going to attempt to figure out what's causing this and how to fix it, but perhaps someone can give me some pointers.

In case it matters, I'm using Visual Studio Express 2013 for Windows Desktop.

/cc @piscisaureus @rvagg

@piscisaureus
Copy link
Contributor

@seishun I have no idea, the same issue is bugging me. A patch (or an explanation) would be very welcome.

@bnoordhuis
Copy link
Member

Does src/node.js or the files in lib/ have timestamps in the future? Or maybe the generated node_natives.h ends up with a timestamp that is in the past somehow? That's the only thing I can think of.

@piscisaureus
Copy link
Contributor

I think I know what happens: vcbuild.bat re-generates the .sln/.vcxproj files which makes msbuild re-run the js2c "custom build step".

A workaround is to call vcbuild noprojgen nosign test. A nicer solution I think would be to split the configuration and build steps into separate batch files, e.g. configure.bat and make.bat.

@seishun
Copy link
Contributor Author

seishun commented Dec 19, 2014

Actually, config.gypi is the culprit here. Its modification date gets bumped during the project generation, and it's listed as an input to the node_js2c target. Now I'm trying to find answers to the following questions:

  • What is the magic that prevents the .sln/.vcxproj files from getting re-generated if no change is required, and can we apply the same magic to config.gypi?
  • Why is config.gypi even required for js2c?

@seishun
Copy link
Contributor Author

seishun commented Dec 19, 2014

Looks like config.gypi was added to the inputs in 95fd517. And there is no explanation. sigh

/cc @TooTallNate

@bnoordhuis
Copy link
Member

@seishun It becomes the process.config variable:

$ out/x64.release/node -p process.config
{ target_defaults: 
   { cflags: [],
     default_configuration: 'Release',
     defines: [ 'OPENSSL_NO_SSL2=1' ],
     include_dirs: [],
     libraries: [] },
  variables: 
   { clang: 0,
     gcc_version: 49,
     host_arch: 'x64',
     icu_small: false,
     node_install_npm: true,
     node_prefix: '/home/bnoordhuis/opt/node',
     node_shared_http_parser: false,
     node_shared_libuv: false,
     node_shared_openssl: false,
     node_shared_v8: false,
     node_shared_zlib: false,
     node_tag: '',
     node_use_dtrace: false,
     node_use_etw: false,
     node_use_mdb: false,
     node_use_openssl: true,
     node_use_perfctr: false,
     openssl_no_asm: 0,
     python: '/usr/bin/python',
     target_arch: 'x64',
     uv_library: 'static_library',
     uv_parent_path: '/deps/uv/',
     uv_use_dtrace: false,
     v8_enable_gdbjit: 0,
     v8_enable_i18n_support: 0,
     v8_no_strict_aliasing: 1,
     v8_optimized_debug: 0,
     v8_random_seed: 0,
     v8_use_snapshot: true,
     want_separate_host_toolset: 0 } }

@seishun
Copy link
Contributor Author

seishun commented Dec 19, 2014

I've found the magic! It's in WriteOnDiff for .sln files and WriteXmlIfChanged for .vcxproj files. Perhaps we could do something similar for config.gypi? Admittedly, it feels kinda hacky.

A nicer solution I think would be to split the configuration and build steps into separate batch files, e.g. configure.bat and make.bat.

I'm not sure how this would help. It would just shift the burden of deciding whether to regenerate project files to the user.

@Fishrock123 Fishrock123 added the windows Issues and PRs related to the Windows platform. label Jan 23, 2015
@piscisaureus
Copy link
Contributor

I've found the magic! It's in WriteOnDiff for .sln files and WriteXmlIfChanged for .vcxproj files. Perhaps we could do something similar for config.gypi? Admittedly, it feels kinda hacky.

So this is your chance to make a PR :)

@seishun seishun self-assigned this Feb 1, 2015
@seishun seishun added the build Issues and PRs related to build files or the CI. label Feb 1, 2015
@seishun
Copy link
Contributor Author

seishun commented Feb 1, 2015

I've discovered that Makefile errors if config.gypi is older than configure.py. If my understanding is correct, this means that if we add WriteOnDiff magic to configure.py, re-running it will not always fix the make error. Sounds bad.

This original issue doesn't affect Linux because it doesn't run configure.py when running make. However, if the project structure (and thus node.gyp) has changed, the Makefile automatically runs gyp_node.py. That doesn't happen when building the VS solution (which is actually one of the selling points of gn - "GN supports automatically re-running itself as needed by Ninja as part of the build").

I'm not sure how to fix this properly.

@seishun seishun removed their assignment Feb 1, 2015
@piscisaureus
Copy link
Contributor

@seishun

In the long run I would like to change the windows build scripts to make configure and make independent steps. But first I have to figure out #530.

If it's not possible to fix it now, feel free to close.

@seishun seishun closed this as completed Feb 2, 2015
jasnell added a commit to jasnell/node that referenced this issue Jul 22, 2017
Fixes: nodejs/http2#179

Was fixing issue nodejs#179 and encountered a segault that was
happening somewhat randomly on session destruction. Both
should be fixed
jasnell added a commit to jasnell/node that referenced this issue Aug 3, 2017
Fixes: nodejs/http2#179

Was fixing issue nodejs#179 and encountered a segault that was
happening somewhat randomly on session destruction. Both
should be fixed
jasnell added a commit that referenced this issue Aug 4, 2017
Fixes: nodejs/http2#179

Was fixing issue #179 and encountered a segault that was
happening somewhat randomly on session destruction. Both
should be fixed

PR-URL: #14239
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
jasnell added a commit to jasnell/node that referenced this issue Aug 13, 2017
Fixes: nodejs/http2#179

Was fixing issue nodejs#179 and encountered a segault that was
happening somewhat randomly on session destruction. Both
should be fixed

PR-URL: nodejs#14239
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
addaleax pushed a commit that referenced this issue Aug 14, 2017
Fixes: nodejs/http2#179

Was fixing issue #179 and encountered a segault that was
happening somewhat randomly on session destruction. Both
should be fixed

Backport-PR-URL: #14813
Backport-Reviewed-By: Anna Henningsen <[email protected]>
Backport-Reviewed-By: Timothy Gu <[email protected]>

PR-URL: #14239
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
eti-p-doray pushed a commit to eti-p-doray/node that referenced this issue May 28, 2024
V8 announced deprecation of the following methods:
 - v8::Objecit::SetAccessor(...) in favor of
   v8::Object::SetNativeDataProperty(...),
 - v8::ObjectTemplate::SetNativeDataProperty(...) with AccessControl
   parameter in favor of
   v8::ObjectTemplate::SetNativeDataProperty(...) without AccessControl
   parameter.

See https://crrev.com/c/5006387.

This CL slightly changes behavior of the following properties:
 - process.debugPort (for worker processes),
 - process.title (for worker processes),
 - process.ppid.

The difference is that they will now behave like a regular writable
JavaScript data properties - in case setter callback is not provided
they will be be reconfigured from a native data property (the one
that calls C++ callbacks upon get/set operations) to a real data
property (so subsequent reads will no longer trigger C++ getter
callbacks).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

4 participants