-
Notifications
You must be signed in to change notification settings - Fork 572
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
Set BUILD_SHARED_LIB to ON by default for Trilinos #811
Comments
@ambrad, this is going to be added to the updated Therefore, can we expand the scope of this to be "Change Trilinos default for BUILD_SHARED_LIBS to ON"? |
Sounds great! I always set it to ON, anyway, which is what made me notice the typo. (I used to add the flag to the bottom of COMMON.config but then just today noticed it was in the comments, but I didn't notice the typo and ended up with a static build when I simply uncommented it.) |
I updated this issue for the new scope. However, I should have just opened and new issue and closed this. I think that makes more sense with GitHub Issues. With Trac tickets, it keeps full history of all changes to every field so you can track any type of change. Not so with GitHub. It is much more limited (and not in a good way in my opinion). |
@bartlettroscoe - we really need to educate some users about shared versus static libraries. On KNL for instance shared libraries can be a disaster because of how Linux maps out the addresses. It would be helpful to look at platforms where we know this is a significant issue and change the default since many users will not actively set this and yet will perform benchmarking. |
I am fine with changing the default for different platforms. We just need to document and make it clear what default is being chosen and on what platform (and why) so that this does not surprise people. |
@bartlettroscoe might you even be able to scan CXXFLAGS to look for platform flags and adjust this setting? I am keen we either get the platform/architecture thing working soon or actively change this setting to avoid a significant set of bad benchmarking results which is not good for Trilinos, our platforms or our users. |
I think an explicit |
@bartlettroscoe sure, I'll help any way I can. What I think I'm hinting at is maybe we should not go to shared until we have this in place since it could represent a fairly serious performance loss. |
I agree with @nmhamster. The only thing I was doing with this issue was pointing out that COMMON.config, which we edit before running checkin-test.py, has a typo: BUILD_SHARED_LIBS is erroneously written as BUILD_SHARED. So right now if you want to run checkin-test.py with shared libs (so the build directory is ~20GB and not > 100GB), you can't forget to fix that typo if you uncomment the line. This issue originally asked that the typo be fixed. One could go further and say that the default for check-in testing only should be BUILD_SHARED_LIBS=ON. I always run the checkin script with it ON because otherwise there's a whole bunch of statically linked exes that take up space and link time. My guess is anybody who regularly uses checkin-test.py does, too. So I would advocate (i) fixing the typo now, (ii) making check-test.py default to BUILD_SHARED_LIBS=ON soon-ish, if others agree, and (iii) deferring the decision for the general default to a later date. |
@ambrad sounds reasonable to me. I just want to avoid any more performance related complaints until we can get this understood. Most recent data shows static versus shared makes considerable difference in end application performance. |
has anyone build with shared libs on for the mostly complete stack in OS-X? On Fri, Nov 11, 2016 at 6:33 AM, Roscoe A. Bartlett <
|
I will test shared libs on OSX with STK as part of #482 (which I am working on right now). If I find a problem I will create a Trilinos GitHub Issue and post a Trac ticket to the STK developers native ticket system. |
ideally building panzer would be the ultimate test .. Well, my ultimate On Fri, Nov 11, 2016 at 7:54 AM, Roscoe A. Bartlett <
|
I build the full PyTrilinos stack on Mac OS X all the time. PyTrilinos requires shared libraries. It does not include STK, though I have successfully built it in the past. |
build puke dump [ 24%] Building CXX object [ 24%] Linking CXX shared library libstk_util_env.dylib Undefined symbols for architecture x86_64: "null_streambuf::null_streambuf()", referenced from:
"null_streambuf::~null_streambuf()", referenced from:
"stk::get_heap_used()", referenced from:
in Trace.cpp.o
"stk::all_reduce_impl(ompi_communicator_t_, unsigned long const_,
unsigned long const_, unsigned long_, unsigned int) in memory_util.cpp.o
unsigned long const_, unsigned long_, unsigned int) in memory_util.cpp.o
unsigned long const_, unsigned long_, unsigned int) in memory_util.cpp.o "stk::all_write_string(ompi_communicator_t*,
"stk::parallel_machine_rank(ompi_communicator_t*)", referenced from:
RuntimeMessage.cpp.o
std::__1::basic_ostringstream<char, std::__1::char_traits, "stk::parallel_machine_size(ompi_communicator_t*)", referenced from:
RuntimeMessage.cpp.o
std::__1::basic_ostringstream<char, std::__1::char_traits,
unsigned long, unsigned long&, unsigned long&, unsigned long&) in "stk::diag::Writer::pop()", referenced from:
"stk::diag::Writer::push()", referenced from:
"stk::diag::Writer::dendl()", referenced from:
"stk::diag::Writer::operator<<(stk::diag::Writer&
in Trace.cpp.o
"stk::diag::operator<<(stk::diag::Writer&, char const*)", referenced from:
in Trace.cpp.o
Trace.cpp.o "stk::diag::operator<<(stk::diag::Writer&, std::__1::basic_string<char,
"stk::Marshal::Marshal(std::__1::basic_string<char,
RuntimeMessage.cpp.o "stk::Marshal::Marshal(unsigned int)", referenced from:
RuntimeMessage.cpp.o "stk::Bootstrap::Bootstrap(void (*)())", referenced from:
"stk::Marshal& stk::operator<<<std::__1::basic_string<char,
namespace)::DeferredMessage const&) in RuntimeMessage.cpp.o "stk::Marshal& stk::operator<<std::type_info(stk::Marshal&,
namespace)::DeferredMessage>(stk::Marshal&, "stk::Marshal& stk::operator<<(stk::Marshal&, int const&)",
namespace)::DeferredMessage const&) in RuntimeMessage.cpp.o "stk::Marshal& stk::operator<<(stk::Marshal&, long const&)",
namespace)::DeferredMessage const&) in RuntimeMessage.cpp.o "stk::Marshal& stk::operator<<(stk::Marshal&, unsigned
namespace)::DeferredMessage>(stk::Marshal&,
namespace)::DeferredMessage const&) in RuntimeMessage.cpp.o "stk::Marshal& stk::operator>><std::type_info const>(stk::Marshal&,
namespace)::DeferredMessage>(stk::Marshal&, "stk::Marshal& stk::operator>><std::__1::basic_string<char,
namespace)::DeferredMessage&) in RuntimeMessage.cpp.o "stk::Marshal& stk::operator>>(stk::Marshal&, int&)", referenced
namespace)::DeferredMessage&) in RuntimeMessage.cpp.o "stk::Marshal& stk::operator>>(stk::Marshal&, long&)", referenced
namespace)::DeferredMessage&) in RuntimeMessage.cpp.o "stk::Marshal& stk::operator>>(stk::Marshal&, unsigned
namespace)::DeferredMessage>(stk::Marshal&,
namespace)::DeferredMessage&) in RuntimeMessage.cpp.o "sierra::SignalHandler::add_handler(std::__1::basic_string<char,
std::__1::char_traits, std::__1::allocator > const&) in "sierra::SignalHandler::check_signal_name(std::__1::basic_string<char,
std::__1::char_traits, std::__1::allocator > const&) in "sierra::SignalHandler::instance()", referenced from:
std::__1::char_traits, std::__1::allocator > const&) in "sierra::Env::HUP_received()", referenced from:
ld: symbol(s) not found for architecture x86_64 clang: error: linker command failed with exit code 1 (use -v to see make[2]: *** make[1]: *** make[1]: *** Waiting for unfinished jobs.... On Fri, Nov 11, 2016 at 8:34 AM, Bill Spotz [email protected]
|
Panzer will be in the new set of Primary Tested (PT) packages and will get tested everywhere (with at least one build). See: I will test a shared build of all of this including Panzer on OSX soon as part of #482 (hopefully later today if things go well on Linux). |
Thanks Ross, if it works, can I get a config file? On Fri, Nov 11, 2016 at 9:13 AM, Roscoe A. Bartlett <
|
The checkin-test-sems.sh script will spit out out automatically. You can copy that and use it for whatever.
The SEMS env. See #158. |
…rilinos#811) This defines a new standard single CI build. What is in the build can be debated and changed but I think what is here is close to the one possible best build that protects the most code, the most development activities, and the most users.
@bathmatt, I can confirm build failures on OSX with shared libraries in this comment in #482. I will put in a more detailed analysis in a following comment in #482 (but this does not look good for allowing people to push to Trilinos from OSX). |
@bathmatt, @bartlettroscoe Some of these STK issues have been known for a while; I had a work-around for a few of them (#62) and can submit a related pull request as @Dalg suggested. Based on the latest comments in (#62) I had been waiting for @bmpersc or @bartlettroscoe to suggest a path forward (either pushing changes to STK or to wait for the FY17 STK restructuring). |
Once I have finished #482, I will submit some new Trilinos GitHub issues for the failures that I am currently seeing on OSX with a standard SEMS env and I will also post Trac tickets to the STK developers' native issue tracking system. This problem can be solved on the Trilinos 'develop' branch without a massive restructuring if some careful usage of git revert is used before syncing back and forth. I will take that up with @bmpersc later. |
…rilinos#811) This defines a new standard single CI build. What is in the build can be debated and changed but I think what is here is close to the one possible best build that protects the most code, the most development activities, and the most users.
This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity. |
This issue was closed due to inactivity for 395 days. |
Description: (written by Ross Bartlett)
By default raw CMake projects don't set
BUILD_SHARED_LIBS
which means it is off and you get static libs. That is bad. In general, static libs take longer to create and link and take up a lot more disk space than for shared libs.Also, shared libs should be the default as per the agreement for the larger xSDK standards that are being developed as part of the IDEAS project (see "Select option used for indicating whether to build shared libraries (default is shared in XSDK mode")).
Also, note that RPATH issues has been resolved (see TriBITSPub/TriBITS#126) and installed libraries and exectuables run without needing to set any env vars.
Therefore, this Story is to change
BUILD_SHARED_LIB
toON
by default for Trilinos.Original Description:
@bartlettroscoe: The file ./cmake/tribits/ci_support/CheckinTest.py generates .config files with example configuration changes commented out. One of them is -DBUILD_SHARED:BOOL=ON. But this should be -DBUILD_SHARED_LIBS:BOOL=ON.
The text was updated successfully, but these errors were encountered: