n-riesco / jp-kernel

Generic Node.js kernel for the Jupyter notebook
Other
21 stars 15 forks source link

Bump jmp@0.7.0 #3

Closed lgeiger closed 7 years ago

lgeiger commented 7 years ago

Bring in https://github.com/zeromq/zeromq.js

n-riesco commented 7 years ago

@lgeiger Could we have 32bit binaries first, please? Otherwise this change is going to cause me some inconvenience (since most of IJavascript development happens in a 32bit machine).

Also, since this is your first contribution to jp-kernel, the AUTHORS file needs updating. You know the drill:

Pull requests will be distributed under the terms in the LICENSE file. Hence, before accepting any pull requests, it is important that the copyright holder of a pull request acknowledges their consent. To express this consent, please, ensure the AUTHORS file has been updated accordingly.

lgeiger commented 7 years ago

Also, since this is your first contribution to jp-kernel, the AUTHORS file needs updating.

@n-riesco Sorry I was working from the github UI and commited to the wrong branch.

Could we have 32bit binaries first, please? Otherwise this change is going to cause me some inconvenience (since most of IJavascript development happens in a 32bit machine).

It will build from source on 32-bit just like it did before. With the only difference being that it uses the bundled ØMQ 4.1.5 instead of the one installed on your system. You don't need any extra dependencies https://github.com/zeromq/zeromq.js#installation---contributors-and-development

In theory we could ship 32-bit binaries right now but we need a 32-bit build machine (or a docker image). Since Travis and Circle CI only have 64-bit VMs we cannot automate this easily (We would need to somehow use docker on Travis). Do you know any build services using 32-bit VMs? If you really want 32-bit prebuilts, you can manually build them and upload them to the latest github release.

n-riesco commented 7 years ago

On 31/10/16 17:31, Lukas Geiger wrote:

Could we have 32bit binaries first, please? Otherwise this change is going to cause me some inconvenience (since most of IJavascript development happens in a 32bit machine).

It will build from source on 32-bit just like it did before. With the only difference being that it uses the bundled ØMQ 4.1.5 instead of the one installed on your system. You don't need any extra dependencies https://github.com/zeromq/zeromq.js#installation---contributors-and-development

I know. I've checked that works. The problem is that it renders my notebook unusable until the compilation of ZMQ completes. I haven't timed it but I reckon that's more than 10'.

I can't deal with this today. Please, don't waste your time on building the 32bit binaries. I'll come back to you on this.

n-riesco commented 7 years ago

@lgeiger I'm now using yarn to cache the installation of zeromq (and this's helped a great deal to alleviate my pain).

I'm still concerned that the use of zeromq in jp-kernel will make me lose most of the 32bit users.

Here are some ideas that would help ease the pain for 32bit users:

lgeiger commented 7 years ago

I haven't timed it but I reckon that's more than 10'.

Ouch 10 minutes? Thats way too long. I timed the libzmq build script on my notebook (OS X) using different settings:

command time
make -j/ make -j 4 24 s
make -j 2 26 s
make 36 s

I have no idea why it takes so long on your machine. You could try to use ccache, maybe it helps a bit.

I'm now using yarn to cache the installation of zeromq (and this's helped a great deal to alleviate my pain).

That's great! But we can not recommend yarn for other platforms right now since it will allays build from source instead of using the prebuilts: https://github.com/mafintosh/prebuild/issues/132

(for 32bit users only) do not compile tweetnacl in parallel (my feeling is that this is the point where my notebook freezes).

We can definitely limit the build to 2 cores since the improvement over a full parallel build (4 virtual cores on my machine) is marginal.

(for 32bit linux users only) use the native zmq library if present

I'm against that since it would introduce the installation problems we had with https://github.com/JustinTulloss/zeromq.node again. If we would dynamically link libzmq we couldn't use zeromq inside a bundled application like nteract.

lgeiger commented 7 years ago

zeromq v3.1.1 is now available.

Should I make a PR to jmp to require to ^3.1.1 explicitly?

n-riesco commented 7 years ago

On 07/11/16 02:39, Lukas Geiger wrote:

|zeromq v3.1.1| is now available.

Should I make a PR to |jmp| to require to |^3.1.1| explicitly?

I haven't come up with anything yet to stop the repeat compilations in 32bit machines?

Any progress on your side?

lgeiger commented 7 years ago

I haven't come up with anything yet to stop the repeat compilations in 32bit machines?

I thought speeding up the build resolved your problem. I'll take a look at this 👍

n-riesco commented 7 years ago

@lgeiger I've just published jmp@0.7.2. Please, could you update this PR to use ^0.7.2?

lgeiger commented 7 years ago

Thanks for releasing jmp 0.7.2 so fast. I updated this PR.

n-riesco commented 7 years ago

@lgeiger Were you reading over my shoulder? :stuck_out_tongue_closed_eyes:

lgeiger commented 7 years ago

😂 GitHub for the win bildschirmfoto 2016-11-07 um 16 41 03