Set BUILD_SHARED_LIB to ON by default for Trilinos

[ambrad]

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 to ON 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.

[bartlettroscoe]

@ambrad, this is going to be added to the updated project-checkin-test-config.py file as part of #482 for the --default-builds which I am working on right now as I write this. However, I will argue that BUILD_SHARED_LIBS should be set to ON by default for all builds, especially now that the RPATH issue has been resolved (see TriBITSPub/TriBITS#126). Also, that was 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")).

Therefore, can we expand the scope of this to be "Change Trilinos default for BUILD_SHARED_LIBS to ON"?

[ambrad]

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.)

[bartlettroscoe]

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).

[nmhamster]

@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.

[bartlettroscoe]

@nmhamster,

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.

[nmhamster]

@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.

[bartlettroscoe]

I think an explicit Trilinos_PLATFORM_ARCH argument is likely the right thing to do. I think we can piggy-back off of the Kokkos makefile system to get the right CXXFLAGS and other options. I have some ideas about how to automatically do that if you are interested. Basically, in at configure time in CMake you just run 'make' on dummy Kokkos client makefile that passed in the right options and extracts out the generated variables. We just need someone with the time and skills to do this (but this is pretty easy CMake stuff). Should we create a new Trilinos GitHub Issue for adding a Trilinos_PLATFORM_ARCH option and discuss the options there?

[nmhamster]

@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.

[ambrad]

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.

[nmhamster]

@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.

[bartlettroscoe]

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.

(i) I fixed the typo and pushed to the TriBITS repo (that is where the checkin-test.py script lives). The next time I snapshot TriBITS to Trilinos, this will get updated.

(ii) I already have BUILD_SHARED_LIBS default to on in CI mode on my branch

(iii) Fine with me. But I think we are hurting the average user when we default to static libs.

Most recent data shows static versus shared makes considerable difference in end application performance.

What applications? What platforms?

[bathmatt]

has anyone build with shared libs on for the mostly complete stack in OS-X?
stk doesn't work for some reason for me, so this will mess with some
people.
Just adding a data point

On Fri, Nov 11, 2016 at 6:33 AM, Roscoe A. Bartlett <
[email protected]> wrote:

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.

(i) I fixed the typo and pushed to the TriBITS repo (that is where the
checkin-test.py script lives). The next time I snapshot TriBITS to
Trilinos, this will get updated.

(ii) I already have BUILD_SHARED_LIBS default to on in CI mode on my
branch
https:/bartlettroscoe/Trilinos/tree/better-ci-build-482

(iii) Fine with me. But I think we are hurting the average user when we
default to static libs.

Most recent data shows static versus shared makes considerable difference
in end application performance.

What applications? What platforms?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#811 (comment),
or mute the thread
https:/notifications/unsubscribe-auth/AOPDIACttAfsm_LW2BS-FPOvpnyPkW6kks5q9G6qgaJpZM4KvLYL
.

[bartlettroscoe]

has anyone build with shared libs on for the mostly complete stack in OS-X? stk doesn't work for some reason for me, so this will mess with some people.

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.

[bathmatt]

ideally building panzer would be the ultimate test .. Well, my ultimate
test at least :)

On Fri, Nov 11, 2016 at 7:54 AM, Roscoe A. Bartlett <
[email protected]> wrote:

has anyone build with shared libs on for the mostly complete stack in
OS-X? stk doesn't work for some reason for me, so this will mess with some
people.

I will test shared libs on OSX with STK as part of #482
#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.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#811 (comment),
or mute the thread
https:/notifications/unsubscribe-auth/AOPDIA2SVrg9dXQRdO5wqJkTz7rP_1Swks5q9IGzgaJpZM4KvLYL
.

[wfspotz]

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.

[bathmatt]

build puke dump

[ 24%] Building CXX object
packages/stk/stk_util/stk_util/diag/CMakeFiles/stk_util_diag.dir/Option.cpp.o

[ 24%] Linking CXX shared library libstk_util_env.dylib

Undefined symbols for architecture x86_64:

"null_streambuf::null_streambuf()", referenced from:

  stk::EnvData::EnvData() in EnvData.cpp.o

"null_streambuf::~null_streambuf()", referenced from:

  stk::EnvData::EnvData() in EnvData.cpp.o

  stk::EnvData::~EnvData() in EnvData.cpp.o

"stk::get_heap_used()", referenced from:

  stk::diag::Trace::Trace(stk::diag::Writer&, char const*, int, bool)

in Trace.cpp.o

  stk::diag::Trace::~Trace() in Trace.cpp.o

"stk::all_reduce_impl(ompi_communicator_t_, unsigned long const_,
unsigned long_, unsigned int, ompi_op_t_)", referenced from:

  void stk::all_reduce_max<unsigned long>(ompi_communicator_t*,

unsigned long const_, unsigned long_, unsigned int) in memory_util.cpp.o

  void stk::all_reduce_min<unsigned long>(ompi_communicator_t*,

unsigned long const_, unsigned long_, unsigned int) in memory_util.cpp.o

  void stk::all_reduce_sum<unsigned long>(ompi_communicator_t*,

unsigned long const_, unsigned long_, unsigned int) in memory_util.cpp.o

"stk::all_write_string(ompi_communicator_t*,
std::__1::basic_ostream<char, std::__1::char_traits >&,
std::__1::basic_string<char, std::__1::char_traits,
std::__1::allocator > const&)", referenced from:

  sierra::Env::output_flush() in Env.cpp.o

"stk::parallel_machine_rank(ompi_communicator_t*)", referenced from:

  stk::report_deferred_messages(ompi_communicator_t*) in

RuntimeMessage.cpp.o

  stk::aggregate_messages(ompi_communicator_t*,

std::__1::basic_ostringstream<char, std::__1::char_traits,
std::__1::allocator >&, char const*) in RuntimeMessage.cpp.o

"stk::parallel_machine_size(ompi_communicator_t*)", referenced from:

  stk::report_deferred_messages(ompi_communicator_t*) in

RuntimeMessage.cpp.o

  stk::aggregate_messages(ompi_communicator_t*,

std::__1::basic_ostringstream<char, std::__1::char_traits,
std::__1::allocator >&, char const*) in RuntimeMessage.cpp.o

  void stk::get_max_min_avg<unsigned long>(ompi_communicator_t*,

unsigned long, unsigned long&, unsigned long&, unsigned long&) in
memory_util.cpp.o

"stk::diag::Writer::pop()", referenced from:

  stk::diag::pop(stk::diag::Writer&) in Trace.cpp.o

"stk::diag::Writer::push()", referenced from:

  stk::diag::push(stk::diag::Writer&) in Trace.cpp.o

"stk::diag::Writer::dendl()", referenced from:

  stk::diag::dendl(stk::diag::Writer&) in Trace.cpp.o

"stk::diag::Writer::operator<<(stk::diag::Writer&
(*)(stk::diag::Writer&))", referenced from:

  stk::diag::Trace::Trace(stk::diag::Writer&, char const*, int, bool)

in Trace.cpp.o

  stk::diag::Trace::~Trace() in Trace.cpp.o

"stk::diag::operator<<(stk::diag::Writer&, char const*)", referenced from:

  stk::diag::Trace::Trace(stk::diag::Writer&, char const*, int, bool)

in Trace.cpp.o

  stk::diag::Trace::~Trace() in Trace.cpp.o

  stk::diag::Trace::verbose_print(stk::diag::Writer&) const in

Trace.cpp.o

"stk::diag::operator<<(stk::diag::Writer&, std::__1::basic_string<char,
std::__1::char_traits, std::__1::allocator > const&)",
referenced from:

  stk::diag::Trace::~Trace() in Trace.cpp.o

"stk::Marshal::Marshal(std::__1::basic_string<char,
std::__1::char_traits, std::__1::allocator > const&)",
referenced from:

  stk::report_deferred_messages(ompi_communicator_t*) in

RuntimeMessage.cpp.o

"stk::Marshal::Marshal(unsigned int)", referenced from:

  stk::report_deferred_messages(ompi_communicator_t*) in

RuntimeMessage.cpp.o

"stk::Bootstrap::Bootstrap(void (*)())", referenced from:

  ___cxx_global_var_init.1 in RuntimeMessage.cpp.o

"stk::Marshal& stk::operator<<<std::__1::basic_string<char,
std::__1::char_traits, std::__1::allocator > >(stk::Marshal&,
std::__1::basic_string<char, std::__1::char_traits,
std::__1::allocator > const&)", referenced from:

  stk::(anonymous namespace)::operator<<(stk::Marshal&, stk::(anonymous

namespace)::DeferredMessage const&) in RuntimeMessage.cpp.o

"stk::Marshal& stk::operator<<std::type_info(stk::Marshal&,
std::type_info const&)", referenced from:

  stk::Marshal& stk::operator<<<stk::(anonymous

namespace)::DeferredMessage>(stk::Marshal&,
std::__1::vector<stk::(anonymous namespace)::DeferredMessage,
std::__1::allocator<stk::(anonymous namespace)::DeferredMessage> > const&)
in RuntimeMessage.cpp.o

"stk::Marshal& stk::operator<<(stk::Marshal&, int const&)",
referenced from:

  stk::(anonymous namespace)::operator<<(stk::Marshal&, stk::(anonymous

namespace)::DeferredMessage const&) in RuntimeMessage.cpp.o

"stk::Marshal& stk::operator<<(stk::Marshal&, long const&)",
referenced from:

  stk::(anonymous namespace)::operator<<(stk::Marshal&, stk::(anonymous

namespace)::DeferredMessage const&) in RuntimeMessage.cpp.o

"stk::Marshal& stk::operator<<(stk::Marshal&, unsigned
long const&)", referenced from:

  stk::Marshal& stk::operator<<<stk::(anonymous

namespace)::DeferredMessage>(stk::Marshal&,
std::__1::vector<stk::(anonymous namespace)::DeferredMessage,
std::__1::allocator<stk::(anonymous namespace)::DeferredMessage> > const&)
in RuntimeMessage.cpp.o

  stk::(anonymous namespace)::operator<<(stk::Marshal&, stk::(anonymous

namespace)::DeferredMessage const&) in RuntimeMessage.cpp.o

"stk::Marshal& stk::operator>><std::type_info const>(stk::Marshal&,
std::type_info const&)", referenced from:

  stk::Marshal& stk::operator>><stk::(anonymous

namespace)::DeferredMessage>(stk::Marshal&,
std::__1::vector<stk::(anonymous namespace)::DeferredMessage,
std::__1::allocator<stk::(anonymous namespace)::DeferredMessage> >&) in
RuntimeMessage.cpp.o

"stk::Marshal& stk::operator>><std::__1::basic_string<char,
std::__1::char_traits, std::__1::allocator > >(stk::Marshal&,
std::__1::basic_string<char, std::__1::char_traits,
std::__1::allocator >&)", referenced from:

  stk::(anonymous namespace)::operator>>(stk::Marshal&, stk::(anonymous

namespace)::DeferredMessage&) in RuntimeMessage.cpp.o

"stk::Marshal& stk::operator>>(stk::Marshal&, int&)", referenced
from:

  stk::(anonymous namespace)::operator>>(stk::Marshal&, stk::(anonymous

namespace)::DeferredMessage&) in RuntimeMessage.cpp.o

"stk::Marshal& stk::operator>>(stk::Marshal&, long&)", referenced
from:

  stk::(anonymous namespace)::operator>>(stk::Marshal&, stk::(anonymous

namespace)::DeferredMessage&) in RuntimeMessage.cpp.o

"stk::Marshal& stk::operator>>(stk::Marshal&, unsigned
long&)", referenced from:

  stk::Marshal& stk::operator>><stk::(anonymous

namespace)::DeferredMessage>(stk::Marshal&,
std::__1::vector<stk::(anonymous namespace)::DeferredMessage,
std::__1::allocator<stk::(anonymous namespace)::DeferredMessage> >&) in
RuntimeMessage.cpp.o

  stk::(anonymous namespace)::operator>>(stk::Marshal&, stk::(anonymous

namespace)::DeferredMessage&) in RuntimeMessage.cpp.o

"sierra::SignalHandler::add_handler(std::__1::basic_string<char,
std::__1::char_traits, std::__1::allocator > const&,
sierra::Callback&)", referenced from:

  stk::util::Scheduler::set_signal(std::__1::basic_string<char,

std::__1::char_traits, std::__1::allocator > const&) in
Scheduler.cpp.o

"sierra::SignalHandler::check_signal_name(std::__1::basic_string<char,
std::__1::char_traits, std::__1::allocator > const&)",
referenced from:

  stk::util::Scheduler::set_signal(std::__1::basic_string<char,

std::__1::char_traits, std::__1::allocator > const&) in
Scheduler.cpp.o

"sierra::SignalHandler::instance()", referenced from:

  stk::util::Scheduler::set_signal(std::__1::basic_string<char,

std::__1::char_traits, std::__1::allocator > const&) in
Scheduler.cpp.o

"sierra::Env::HUP_received()", referenced from:

  sierra::Env::is_shutdown_requested() in Env.cpp.o

ld: symbol(s) not found for architecture x86_64

clang: error: linker command failed with exit code 1 (use -v to see
invocation)

make[2]: ***
[packages/stk/stk_util/stk_util/environment/libstk_util_env.12.9.dylib]
Error 1

make[1]: ***
[packages/stk/stk_util/stk_util/environment/CMakeFiles/stk_util_env.dir/all]
Error 2

make[1]: *** Waiting for unfinished jobs....

On Fri, Nov 11, 2016 at 8:34 AM, Bill Spotz [email protected]
wrote:

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.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#811 (comment),
or mute the thread
https:/notifications/unsubscribe-auth/AOPDIDd1BajgmOTpguU1qjAlt9Be4gXgks5q9Ir5gaJpZM4KvLYL
.

[bartlettroscoe]

ideally building panzer would be the ultimate test .. Well, my ultimate test at least :)

Panzer will be in the new set of Primary Tested (PT) packages and will get tested everywhere (with at least one build). See:

• Initial adjustment of Primary Tested set of Trilinos Packages and TPLs #410 (comment)

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).

[bathmatt]

Thanks Ross, if it works, can I get a config file?
What are you using for TPLS, i.e., what source? Brew/macports?

On Fri, Nov 11, 2016 at 9:13 AM, Roscoe A. Bartlett <
[email protected]> wrote:

ideally building panzer would be the ultimate test .. Well, my ultimate
test at least :)

Panzer will be in the new set of Primary Tested (PT) packages and will get
tested everywhere (with at least one build). See:

• Initial adjustment of Primary Tested set of Trilinos Packages and TPLs #410 (comment)
  Initial adjustment of Primary Tested set of Trilinos Packages and TPLs #410 (comment)

I will test a shared build of all of this including Panzer on OSX soon as
part of #482 #482 (hopefully
later today if things go well on Linux).

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#811 (comment),
or mute the thread
https:/notifications/unsubscribe-auth/AOPDIFL3lOrkwxcWkULk93xtv1iJii8Dks5q9JQ0gaJpZM4KvLYL
.

[bartlettroscoe]

if it works, can I get a config file?

The checkin-test-sems.sh script will spit out out automatically. You can copy that and use it for whatever.

What are you using for TPLS, i.e., what source? Brew/macports?

The SEMS env. See #158.

[bartlettroscoe]

has anyone build with shared libs on for the mostly complete stack in OS-X? stk doesn't work for some reason for me, so this will mess with some people. Just adding a data point

@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).

[pbosler]

@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).

[bartlettroscoe]

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.

[github-actions]

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.
If you would like to keep this issue open please add a comment and remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.

[github-actions]

This issue was closed due to inactivity for 395 days.