lilypond-devel
[Top][All Lists]
Advanced

[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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]