Reviewers: lemzwerg, Dan Eble, Message: On 2020/01/27 21:05:27, Dan Eble wrote: > > 1) Drop exception handling around scm_boot_guile > > Doing this at the end of main() is pointless. > > Agreed. (It should have been caught by reference anyway.) > > > 2) Disable exception handling code > > We don't throw in LilyPond and it's only bloating the executable: > > before: 192,586,512B > > after: 166,845,952B (-13.3%) > > A. Do we care that much about about 30 MB? Is someone complaining?
Yes, I care. Why waste space if we don't have to? > B. The standard library may throw for various reasons. As far as I know, it calls abort() instead of raising exceptions when compiled with -fno-exceptions. Even if not, libc will catch the exception and show the result of e.what(). > > This might also have a positive effect on the build time > > I have no experience with this. Short unscientific experiment on my otherwise idle laptop from 2016 (make -j4): before: real 6m34,192s user 20m12,628s sys 1m18,162s after: real 6m12,393s user 18m48,599s sys 1m15,815s > > as well as performance during runtime > > Not likely. The cost is about zero until one is thrown. I've worked on > high-availability, high-performance network packet processing products where we > used exceptions to good effect. Agreed with the generated exception handling code on Linux, I might have confused with the older version from Windows. But even they have a low overhead nowadays. > I don't have a better reason to oppose disabling exceptions than that I've found > them useful in other projects, so I'll say LGTM; but I think we could offer > posterity a better justification than 30 MB and maybe build time. > > To me, the most convincing argument for disabling exceptions (which in g++ turns > them into abort(), IIUC; I'm not sure about clang) would be that you don't want > LilyPond contributors to have to write exception-safe code. It's not natural > for C programmers coming into C++. Also, I'm sure you've already got some. Just my opinion: I think many projects work well without exceptions, with GCC and LLVM being two large ones that I'm used to. At the moment, I don't see a valid use case for exceptions in LilyPond, but I haven't seen all of the code yet. If exceptions prove helpful in the future, we can certainly think about switching them back on. Description: Disable C++ exceptions 1) Drop exception handling around scm_boot_guile Doing this at the end of main() is pointless. 2) Disable exception handling code We don't throw in LilyPond and it's only bloating the executable: before: 192,586,512B after: 166,845,952B (-13.3%) This might also have a positive effect on the build time as well as performance during runtime, but I didn't try to measure that for now. Please review this at https://codereview.appspot.com/571430048/ Affected files (+4, -16 lines): M lily/main.cc M stepmake/stepmake/c++-vars.make M stepmake/stepmake/test-rules.make Index: lily/main.cc diff --git a/lily/main.cc b/lily/main.cc index b23c1596e7b3d1d0247e5a6664fae1bca4e48b53..a8965712bcd0a2974866392418400b88aee79f9c 100644 --- a/lily/main.cc +++ b/lily/main.cc @@ -818,22 +818,7 @@ main (int argc, char **argv, char **envp) /* * Start up Guile API using main_with_guile as a callback. */ -#if (GUILEV2) - /* Debugging aid. - Set it on by default for Guile V2 - while migration in progress. - */ - try - { - scm_boot_guile (argc, argv, main_with_guile, 0); - } - catch (std::exception e) - { - error (_f ("exception caught: %s", e.what ())); - }; -#else scm_boot_guile (argc, argv, main_with_guile, 0); -#endif /* Only reachable if GUILE exits. That is an error. */ return 1; Index: stepmake/stepmake/c++-vars.make diff --git a/stepmake/stepmake/c++-vars.make b/stepmake/stepmake/c++-vars.make index 73dd91e240170d11f7736424e7da81ba6c21453a..9a011d3fb9ef8761f200d67ed73ffff58084b810 100644 --- a/stepmake/stepmake/c++-vars.make +++ b/stepmake/stepmake/c++-vars.make @@ -14,7 +14,7 @@ DO_O_DEP = rm -f $(o-dep-out); DEPENDENCIES_OUTPUT="$(o-dep-out) $(outdir)/$(not lo-dep-out = $(outdir)/$(subst .lo,.dep,$(notdir $@))# DO_LO_DEP = rm -f $(lo-dep-out); DEPENDENCIES_OUTPUT="$(lo-dep-out) $(outdir)/$(notdir $@)" -EXTRA_CXXFLAGS = -std=c++11 -W -Wall -Wconversion -Woverloaded-virtual +EXTRA_CXXFLAGS = -std=c++11 -fno-exceptions -W -Wall -Wconversion -Woverloaded-virtual #ifeq ($(MY_PATCH_LEVEL),) #EXTRA_CXXFLAGS += -Werror #endif Index: stepmake/stepmake/test-rules.make diff --git a/stepmake/stepmake/test-rules.make b/stepmake/stepmake/test-rules.make index 63276b9e5c2385fd5fa8160166d61c11e0edd654..7485dc1a5a5bb4b4a702aad96b2d3dd065289e45 100644 --- a/stepmake/stepmake/test-rules.make +++ b/stepmake/stepmake/test-rules.make @@ -6,6 +6,9 @@ endef $(foreach a, $(MODULE_LIBS), $(eval $(call MODULE_LIB_template,$(a)))) +# yaffut.hh catches all exceptions, so re-enable -fexceptions for the tests. +$(TEST_O_FILES): EXTRA_CXXFLAGS += -fexceptions + $(TEST_EXECUTABLE): $(TEST_O_FILES) $(TEST_MODULE_LIBS:%=%/$(outdir)/library.a) $(call ly_progress,Making,$@,) $(foreach a, $(TEST_MODULE_LIBS), $(MAKE) -C $(a) && ) true
