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

Add prebuilds #992

Merged
merged 1 commit into from
Mar 7, 2018
Merged

Add prebuilds #992

merged 1 commit into from
Mar 7, 2018

Conversation

chearon
Copy link
Collaborator

@chearon chearon commented Sep 7, 2017

canvas-prebuilt has been working for a year now, and the latest version of the Linux build has broad compatibility. This would try to download the binaries from the prebuilt repo before node-gyp and hopefully cut down on a lot of installation issues.

You can check out this branch and npm install to try it out.

If we do go forward with this I can give access to @LinusU and @zbjornson and maybe others to the prebuilt repo, and document the release process better. It basically involves setting environment variables to indicate which version you want to build and release, and then you re-build the latest commit on AppVeyor/Travis.

(We could also use prebuild-install, it has half the dependencies which is nice. But (1) node-pre-gyp is more popular and thus more likely to already be installed and (2) prebuild-install is not compatible with canvas-prebuilt's tarball structure (fixable for later versions)).

edit: Forgot this affects CI too, will fix that soon fixed, but CI is stalled now CI fixed

@chearon chearon force-pushed the prebuild branch 2 times, most recently from 6ee7458 to 34630b5 Compare September 7, 2017 02:08
@zbjornson
Copy link
Collaborator

(Would be nifty to setup a release hook that kicks off the prebuild process sometime.)

Does the readme need to be updated to say something like "prebuilt will be used by default; if you want to build from source, use --build-from-source and install all these dependencies"?

@chearon
Copy link
Collaborator Author

chearon commented Sep 7, 2017

(Would be nifty to setup a release hook that kicks off the prebuild process sometime.)

Definitely, I might have to set something up that builds the latest version if triggered from a URL. Then just add a web hook to automattic/node-canvas. The environment variables are useful for going back and updating binaries for old versions (e.g. node 9 released).

I'll update the readme.

@LinusU
Copy link
Collaborator

LinusU commented Sep 9, 2017

🙌

Awesome, would you mind writing down some quick notes on how the releases process works?

I created a node-gfx organization on Github where we could potentially put stuff like that repo? If you think it's a good idea... We could also start breaking out some other parts of canvas there (eg. font string parsing, the matrix transform classes, etc.) 🤔

@chearon
Copy link
Collaborator Author

chearon commented Sep 9, 2017

Sure! I'll document it in a wiki. I'm okay with moving the repo as long as GH redirects old release tarball links correctly. I'm going to test that out now. If it doesn't, I'll have to fork a new repo to node-gfx instead.

@LinusU
Copy link
Collaborator

LinusU commented Sep 9, 2017

Great :)

I added both you and @zbjornson as owners, I'd be happy to add anyone one else that's been contributing to node-canvas as well!

Also, I just made the name up on the spot, if anyone have some better ideas we could of course roll with that 🙌

@chearon
Copy link
Collaborator Author

chearon commented Sep 11, 2017

It works! Good on you, GitHub. I'm moving it now and then I'll update the PR.

wget https://github.com/chearon/test-repo/releases/download/v1.0.0/canvas-prebuilt-v2.0.0-alpha.5-node-v57-darwin-x64.tar.gz
--2017-09-11 11:19:36--  https://github.com/chearon/test-repo/releases/download/v1.0.0/canvas-prebuilt-v2.0.0-alpha.5-node-v57-darwin-x64.tar.gz
Resolving github.com... 192.30.253.113, 192.30.253.112
Connecting to github.com|192.30.253.113|:443... connected.
HTTP request sent, awaiting response... 301 Moved Permanently
Location: https://github.com/node-gfx/test-repo/releases/download/v1.0.0/canvas-prebuilt-v2.0.0-alpha.5-node-v57-darwin-x64.tar.gz [following]
--2017-09-11 11:19:37--  https://github.com/node-gfx/test-repo/releases/download/v1.0.0/canvas-prebuilt-v2.0.0-alpha.5-node-v57-darwin-x64.tar.gz
Reusing existing connection to github.com:443.
HTTP request sent, awaiting response... 302 Found

@zbjornson
Copy link
Collaborator

I created a node-gfx organization on Github where we could potentially put stuff like that repo? If you think it's a good idea... We could also start breaking out some other parts of canvas there (eg. font string parsing, the matrix transform classes, etc.)

Good idea! If we end up with pluggable encoders/decoders (e.g. WebP and anything else beyond the Canvas spec requirements), that could be a good place to put them as well. (Could even be a new home for node-canvas if no one at automattic wants to "own" this repo anymore and deal with admin stuff like authorizing appveyor.)

@chearon
Copy link
Collaborator Author

chearon commented Nov 4, 2017

Finally created a wiki here to document the release process. If we were to start using this repo to build releases, you could ignore the Releasing to NPM section. Let me know if anything is unclear, I think I covered everything.

@zbjornson
Copy link
Collaborator

@LinusU are you cool with merging this? :)

@LinusU
Copy link
Collaborator

LinusU commented Mar 7, 2018

Absolutely 👍 🚀 🎉

@LinusU LinusU merged commit 41a835d into Automattic:master Mar 7, 2018
@LinusU
Copy link
Collaborator

LinusU commented Mar 7, 2018

Published as canvas@2.0.0-alpha.10 🎉

@piranna
Copy link
Contributor

piranna commented Mar 7, 2018

Can this allow to move forward to static builds? I'm busy lately, but can be able to upgrade my pull-requests if needed...

@chearon
Copy link
Collaborator Author

chearon commented Mar 7, 2018

Static builds might still be a good idea to avoid issues like node-gfx/node-canvas-prebuilt#30 and #930. I think I've convinced myself that the node-canvas repo should build against system libraries, though, because that + Gyp (CMake in the future?) gives end-users a standardized, easier to understand build.

@LinusU
Copy link
Collaborator

LinusU commented Mar 8, 2018

Yeah, static builds are nice 👍

Personally, I'm super interested in shipping the entire module as a wasm file 😍 but that's probably a bit into the future...

@piranna
Copy link
Contributor

piranna commented Mar 8, 2018

Static builds might still be a good idea to avoid issues like node-gfx/node-canvas-prebuilt#30 and #930

Yeah, and also the problems regarding to OSX and Windows builds due the need to install libraries and so.

Personally, I'm super interested in shipping the entire module as a wasm file but that's probably a bit into the future...

That would be freaking cool! :-D I'm also looking about distribution of Node.js compiled modules as wasm files, no need of build environment for the npm install command :-) Does anybody know if wasm files can be able to load .so files themselves? Since they are linked against custom C libs, maybe it's just a matter of calling dlopen()...

@chearon chearon deleted the prebuild branch March 8, 2018 15:50
@chearon
Copy link
Collaborator Author

chearon commented Mar 8, 2018

Heh, we're all dreaming of no native dependencies.

Someone made an implementation of Canvas in pure JS but without GPU it's just not practical.

If Node gave us a way to communicate with the GPU someone could compile Cairo to WASM. HarfBuzz (font shaping) is already probably not that hard to compile to WASM, but then you have to do FreeType (glyph rendering) too and something for font selection, which could be done in JS since I think the spec is loose on that.

Does anybody know if wasm files can be able to load .so files themselves?

That would be cool, but how do we know the system has the library or has the right version? Isn't it just offloading the problem from build to runtime?

@LinusU
Copy link
Collaborator

LinusU commented Mar 8, 2018

Does anybody know if wasm files can be able to load .so files themselves?

Not directly, although you could probably expose dlopen to the wasm module if you wanted to.

If Node gave us a way to communicate with the GPU someone could compile Cairo to WASM.

Is Cairo GPU accelerated? I didn't think it was 🤔

I think that WebGL integration is planned for WASM, but that is probably quite a while away, and it's also not clear if Node.js would choose to have that support... But if Cairo is just running on the CPU, we probably would be able to get comparable speeds with pure WASM.

@zbjornson
Copy link
Collaborator

Is Cairo GPU accelerated? I didn't think it was

It isn't by default, but can use OpenGL.

#909 intends to add some x86 intrinsics that WASM doesn't support, btw.


My main issue with static builds is that they increase memory usage and occupy more of the CPU cache when the module is loaded by multiple node.js processes -- common because of node's native cluster module. I'd really want it to be an optional linking mode.

@LinusU
Copy link
Collaborator

LinusU commented Mar 9, 2018

[...] increase memory usage and occupy more of the CPU cache when the module is loaded by multiple node.js processes -- common because of node's native cluster module.

Very good point 👍

@piranna
Copy link
Contributor

piranna commented Mar 10, 2018

Someone made an implementation of Canvas in pure JS but without GPU it's just not practical.

I don't think so, JIT can greatly improve performance, so for headless code I find it a good alternative. GPU would improve performance, but I don't find it something needed except for intensive calculations or drawing to an screen.

If Node gave us a way to communicate with the GPU someone could compile Cairo to WASM

Node.js bindings for libdrm, anyone? :-)

Isn't it just offloading the problem from build to runtime?

I think this already is happening on load (aka. runtime) time with dynamic libraries...

Not directly, although you could probably expose dlopen to the wasm module if you wanted to.

That would be a cool project :-) Sort of like FFI, but having WASM just only floats and integers, it would be fairly easier.

Is Cairo GPU accelerated? I didn't think it was

Maybe just only optionally for graphic backends, not for in-memory rendering... Or use the GPU and later fetch the rendered image... Definitely I don't think it's working by default but instead is an opt-in, so probably we are not using it.

I think that WebGL integration is planned for WASM, but that is probably quite a while away, and it's also not clear if Node.js would choose to have that support...

If they add WebGL support it would be API level, not in the core, so Node.js will not have anything of this and would need to be done in an external module.

But if Cairo is just running on the CPU, we probably would be able to get comparable speeds with pure WASM.

I think so.

My main issue with static builds is that they increase memory usage and occupy more of the CPU cache when the module is loaded by multiple node.js processes -- common because of node's native cluster module.

That's not really a problem because the operating system already have the compiled modules in the cache, so the only situation where it would be duplicated it's if you are using statically linked node-canvas and at the same time libcairo in another process, and if so it would only be that they are duplicated, no more. Maybe an increase of about 7 MB in RAM in total due to the duplicated libraries, independently of the Node.js instances running due to the cluster module.

I'd really want it to be an optional linking mode.

Yes, that has been my idea from the begin, that static linking would only be done when dynamic libraries are not installed on the system or when enabled explicitly by hand.

bgw added a commit to bgw/xterm.js that referenced this pull request Mar 29, 2018
This is at least a first step towards fixing xtermjs#1247. This adds some
complexity to the initial setup, since canvas needs to be built from
source.

This won't be true once canvas 2.x is stabilized, because that version
downloads (Automattic/node-canvas#992). JSDom doesn't support 2.x, and
doesn't until a stable release is cut.

We could use
[canvas-prebuilt](https://github.com/node-gfx/node-canvas-prebuilt),
which JSDom does appear to support. However, I'm not sure if that would
cause pain for people with 32-bit operating systems (see the
compatibility table on that page).

I'm not sure if this travis configuration will work; I'll iterate on it
in the PR if it fails. I removed a bunch of hacks that were added
in xtermjs#690, since they don't look necessary anymore (the sleep module is
gone).
bgw added a commit to bgw/xterm.js that referenced this pull request Apr 18, 2018
This is at least a first step towards fixing xtermjs#1247. This adds some
complexity to the initial setup, since canvas needs to be built from
source.

This won't be true once canvas 2.x is stabilized, because that version
downloads (Automattic/node-canvas#992). JSDom doesn't support 2.x, and
doesn't plan to (jsdom/jsdom#1964) until a stable release is cut.

We could use
[canvas-prebuilt](https://github.com/node-gfx/node-canvas-prebuilt),
which JSDom does appear to support. However, I'm not sure if that would
cause pain for people with 32-bit operating systems (see the
compatibility table on that page).

I'm not sure if this travis configuration will work; I'll iterate on it
in the PR if it fails. I removed a bunch of hacks that were added
in xtermjs#690, since they don't look necessary anymore (the sleep module is
gone).
bgw added a commit to bgw/xterm.js that referenced this pull request Apr 18, 2018
This is at least a first step towards fixing xtermjs#1247. This adds some
complexity to the initial setup, since canvas needs to be built from
source.

This won't be true once canvas 2.x is stabilized, because that version
downloads (Automattic/node-canvas#992). JSDom doesn't support 2.x, and
doesn't plan to (jsdom/jsdom#1964) until a stable release is cut.

We could use
[canvas-prebuilt](https://github.com/node-gfx/node-canvas-prebuilt),
which JSDom does appear to support. However, I'm not sure if that would
cause pain for people with 32-bit operating systems (see the
compatibility table on that page).

I'm not sure if this travis configuration will work; I'll iterate on it
in the PR if it fails. I removed a bunch of hacks that were added
in xtermjs#690, since they don't look necessary anymore (the sleep module is
gone).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants