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

build: build release/CI on non-Windows with code cache #22934

Closed
wants to merge 1 commit into from

Conversation

joyeecheung
Copy link
Member

This is the minimal change to enable code cache in
CI/release builds on non-Windows.
(Needs another PR to vcbuild.bat to enable it on Windows).

The normal dev flow is unaltered because currently
make test does not run configure automatically.
To enable code cache locally (so that the tests run faster) one will
need to run make with-code-cache first to persist the
configure options to ./config.status. If that target has
never been run, the build flow is the same as before except
that the code cache test will be skipped instead of being ignored.

Refs: #21563

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

This is the minimal change to enable code cache in
CI/release builds on non-Windows.
(Needs another PR to vcbuild.bat to enable it on Windows).

The normal dev flow is unaltered because currently
`make test` does not run configure automatically.
To enable code cache locally (so that the tests run faster) one will
need to run `make with-code-cache` first to persist the
configure options to `./config.status`. If that target has
never been run, the build flow is the same as before except
that the code cache test will be skipped instead of being ignored.
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Sep 18, 2018
@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung added the wip Issues and PRs that are still a work in progress. label Sep 18, 2018
@joyeecheung
Copy link
Member Author

The corss-compile job wasn't happy, could be the CONFIG_FLAGS in the CI is in conflict with the changes here in the Makefile. It maybe more viable to do this change in Jenkins.

@richardlau
Copy link
Member

The corss-compile job wasn't happy, could be the CONFIG_FLAGS in the CI is in conflict with the changes here in the Makefile. It maybe more viable to do this change in Jenkins.

Given that the job is cross compiling, do we expect the compiled node binary to be runnable on the host?

@richardlau
Copy link
Member

Based on https://ci.nodejs.org/job/node-cross-compile/18938/nodes=cross-compiler-armv6-gcc-4.9.4/console

out/Release/node --expose-internals tools/generate_code_cache.js out/Release/obj/gen/node_code_cache.cc
out/Release/node: 1: out/Release/node: Syntax error: word unexpected (expecting ")")
make[1]: *** [generate-code-cache] Error 2
make[1]: Leaving directory `/mnt/workspace/node-cross-compile'

From
https://ci.nodejs.org/job/node-cross-compile/nodes=cross-compiler-armv6-gcc-4.9.4/ws/config.gypi/*view*/ (link will get overwritten the next time the job runs):

'host_arch': 'ia32',
...
'target_arch': 'arm',

So out/Release/node is compiled for arm and is not runnable on the host (node-msft-cross-compiler-1)

@richardlau
Copy link
Member

https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1604_sharedlibs_withoutintl_x64/7178/console also fails with:

14:11:10 Generated cache for 'https', size = 4.18KB, hash = a3c2d2dfbfc0e2c3938d7c1e1416c72c487ad529773b966b5d209d231156792d, total = 164.16KB
14:11:10 inspector.js:18
14:11:10   throw new ERR_INSPECTOR_NOT_AVAILABLE();
14:11:10   ^
14:11:10 
14:11:10 Error [ERR_INSPECTOR_NOT_AVAILABLE]: Inspector is not available
14:11:10     at inspector.js:18:9
14:11:10     at NativeModule.compile (internal/bootstrap/loaders.js:297:7)
14:11:10     at Function.NativeModule.require (internal/bootstrap/loaders.js:166:18)
14:11:10     at getCodeCache (internal/bootstrap/cache.js:19:16)
14:11:10     at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_withoutintl_x64/tools/generate_code_cache.js:90:22)
14:11:10     at Module._compile (internal/modules/cjs/loader.js:703:30)
14:11:10     at Object.Module._extensions..js (internal/modules/cjs/loader.js:714:10)
14:11:10     at Module.load (internal/modules/cjs/loader.js:613:32)
14:11:10     at tryModuleLoad (internal/modules/cjs/loader.js:552:12)
14:11:10     at Function.Module._load (internal/modules/cjs/loader.js:544:3)
14:11:10 Makefile:115: recipe for target 'generate-code-cache' failed
14:11:10 make[1]: *** [generate-code-cache] Error 1

(the inspector is not expected to be available with non-intl builds).

@joyeecheung
Copy link
Member Author

Ah right, the cross-compile jobs runs build-ci in one go on the host...
The easiest way out may be to opt out of this on those hosts, since it does not seem to be feasible to build that again on the targets.

@joyeecheung
Copy link
Member Author

So..two (better?) solutions:

  1. Try to do the two-pass thing in gyp (is this where you are going with [canary] GYP v8 embedded builtins #22920 ? @refack )
  2. Maybe the code cache is platform independent? ( @nodejs/v8 )? Then we probably can build a binary with the host toolchain to produce the cache..but it probably doesn't really worth a 3-pass build for testing CI, so at best we could do it in release builds, or just forget about the code cache for the cross compiled builds.

(Of course there is always the option of refactoring the node.cc so that we can build a separate executable to produce the code cache, but that's probably not going to happen anytime soon)

@hashseed
Copy link
Member

hashseed commented Oct 2, 2018

Code cache is not platform-independent, since object layout is different between 32-bit and 64-bit platforms.

That said, you can definite cross-compile since whatever your setup is, already configures the host to run V8 with simulator to be able to produce the correct startup snapshot.

@refack
Copy link
Contributor

refack commented Oct 3, 2018

  1. Try to do the two-pass thing in gyp (is this where you are going with [canary] GYP v8 embedded builtins #22920 ? refack

The V8 is build as such. There is v8_base that has all the code. Then a utility exe is made (mksnapshot) that now (since 7.1) produces both the code snapshot, and the embedded-builtins code. Then there is a final target v8 that wraps everything together.
actually there is another step, but it doesn't change the point

I'm planning on PRing a similar structure for node (for example that way node_base could be compiled without the dependencies, for a quick "will this compile" check).

I'll put this high in the backlog.

@refack refack self-assigned this Oct 3, 2018
@joyeecheung
Copy link
Member Author

joyeecheung commented Oct 4, 2018

already configures the host to run V8 with simulator to be able to produce the correct startup snapshot.

That's what I've been wondering about..like how we managed to run mksnapshot in our cross-compile hosts? (I guess the details are in some gyp files, because the hosts just call make build-ci) @refack

@hashseed
Copy link
Member

hashseed commented Oct 4, 2018

That's what I've been wondering about..like how we managed to run mksnapshot in our cross-compile hosts? (I guess the details are in some gyp files, because the hosts just call make build-ci)

If you are for example cross-compiling V8 on your Intel desktop for arm32, then

  • v8_base and mksnapshot are built for ia32, but with simulator for arm32
  • mksnapshot is run. V8 generates JIT code for arm32, but that works because it runs on top of the simulator. This results in the snapshot we need on the target.
  • We then build all of V8 for arm32.
  • The resulting binary and the snapshot are shipped to the target for execution.

@joyeecheung
Copy link
Member Author

Superseded by #27161

@refack
Copy link
Contributor

refack commented Apr 17, 2019

We still need to activate this for the ARM cross compiled binaries. I think it will just work, but it will need to build everything for the host as well as for the target, and I'm worried about CI times 🤷‍♂️

@refack refack removed their assignment Apr 19, 2019
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. wip Issues and PRs that are still a work in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants