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

Reduce package size #86

Merged
merged 2 commits into from
Nov 1, 2016
Merged

Reduce package size #86

merged 2 commits into from
Nov 1, 2016

Conversation

lgeiger
Copy link
Member

@lgeiger lgeiger commented Nov 1, 2016

This PR introduces two changes:

  • Build libzmq with 2 cores only: I timed it here and it doesn't make sense to use more on a average machine.
  • Remove libzmq source after build: This will reduce the package size by about 16.5 MB. I'm leaving the .tar.gz file so it doesn't have to be downloaded again on rebuilding (795KB).

/cc @rgbkrk @n-riesco

@codecov-io
Copy link

Current coverage is 91.16% (diff: 100%)

Merging #86 into master will not change coverage

Powered by Codecov. Last update 6b7ba93...44269ad

@rgbkrk rgbkrk merged commit 7dafaa5 into zeromq:master Nov 1, 2016
@rgbkrk
Copy link
Member

rgbkrk commented Nov 1, 2016

This is helpful for electron and for when this is used in Docker or other container environments.

@lgeiger lgeiger deleted the size branch November 1, 2016 14:00
@n-riesco
Copy link
Contributor

n-riesco commented Nov 1, 2016

I've just tested this PR on a 32bit notebook. The notebook freezes no more and the compilation time has been reduced:

$ time node_module/.bin/prebuild --install
[...]

real    4m6.572s
user    6m28.968s
sys     0m32.784s

@lgeiger
Copy link
Member Author

lgeiger commented Nov 1, 2016

@n-riesco that's great!

Would we get even better times if we use make instead of make -j 2 on your system?

@n-riesco
Copy link
Contributor

n-riesco commented Nov 1, 2016

I get a longer time (but bear in mind that the proper way to do a comparison like this would require running both tests a number of times):

$ git diff
diff --git a/build_libzmq.sh b/build_libzmq.sh
index bde4101..0dc33dd 100755
--- a/build_libzmq.sh
+++ b/build_libzmq.sh
@@ -23,7 +23,7 @@ cd $ZMQ_SRC_DIR

 test -f configure || ./autogen.sh
 ./configure --prefix=$ZMQ_PREFIX --with-relaxed --enable-static --disable-shared
-make -j 2
+make
 make install

 cd $ZMQ_PREFIX


$ time node_modules/.bin/prebuild --install
[...]

real    7m6.190s
user    6m15.212s
sys 0m36.044s

@n-riesco
Copy link
Contributor

n-riesco commented Nov 1, 2016

This commit however is going to make my yarn times longer, because it'll delete all the object files from the cache.

@lgeiger
Copy link
Member Author

lgeiger commented Nov 1, 2016

@n-riesco Thanks for testing. This confirmed my guess that we hit the sweet spot with make -j 2.

This commit however is going to make my yarn times longer, because it'll delete all the object files from the cache.

That's kind of expected. But I prefer a smaller package size over a faster build considering that users likely won't rebuild zeromq very often.

@rgbkrk What do you think?

@rgbkrk
Copy link
Member

rgbkrk commented Nov 1, 2016

A smaller package size certainly helps us in the Electron and Atom case.

An aside: At least on the nteract notebook, we can't use yarn until they support rebuilds (for electron).

What packages are you using yarn with? IJavaScript?

@n-riesco
Copy link
Contributor

n-riesco commented Nov 1, 2016

@rgbkrk I've only started using yarn so that I could test this package in my notebook.

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