[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Disable C++ exceptions (issue 571430048 by address@hidden)
From: |
jonas . hahnfeld |
Subject: |
Re: Disable C++ exceptions (issue 571430048 by address@hidden) |
Date: |
Tue, 28 Jan 2020 09:36:16 -0800 |
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