Skip to content
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

Emit vtables once #521

Merged
merged 1 commit into from
Sep 4, 2020
Merged

Emit vtables once #521

merged 1 commit into from
Sep 4, 2020

Conversation

chfast
Copy link
Collaborator

@chfast chfast commented Sep 4, 2020

This fixes -Wweak-vtable warnings, helpful for #508.

The effect in fizzy library is the following: vtables and typeinfos (V) are generates once in the exceptions.cpp.o instead of all files they were used. And the destructors are not inline any more (W -> T). This is good change as they are always cold.

I'm not convinced the impact is worth the trouble.

diff --git a/names.old b/names.fixed
index da7e6d25..47c47ba0 100644
--- a/names.old
+++ b/names.fixed
@@ -1,0 +2,20 @@
+exceptions.cpp.o:
+0000000000000060 T fizzy::parser_error::~parser_error()
+0000000000000000 T fizzy::parser_error::~parser_error()
+0000000000000000 T fizzy::parser_error::~parser_error()
+0000000000000090 T fizzy::validation_error::~validation_error()
+0000000000000020 T fizzy::validation_error::~validation_error()
+0000000000000020 T fizzy::validation_error::~validation_error()
+00000000000000c0 T fizzy::instantiate_error::~instantiate_error()
+0000000000000040 T fizzy::instantiate_error::~instantiate_error()
+0000000000000040 T fizzy::instantiate_error::~instantiate_error()
+0000000000000000 V typeinfo for fizzy::parser_error
+0000000000000000 V typeinfo for fizzy::validation_error
+0000000000000000 V typeinfo for fizzy::instantiate_error
+0000000000000000 V typeinfo name for fizzy::parser_error
+0000000000000000 V typeinfo name for fizzy::validation_error
+0000000000000000 V typeinfo name for fizzy::instantiate_error
+0000000000000000 V vtable for fizzy::parser_error
+0000000000000000 V vtable for fizzy::validation_error
+0000000000000000 V vtable for fizzy::instantiate_error
+
@@ -8,3 +27,0 @@ execute.cpp.o:
-0000000000000000 W fizzy::instantiate_error::~instantiate_error()
-0000000000000000 W fizzy::instantiate_error::~instantiate_error()
-0000000000000000 W fizzy::instantiate_error::~instantiate_error()
@@ -44 +60,0 @@ execute.cpp.o:
-0000000000000000 V typeinfo for fizzy::instantiate_error
@@ -48 +63,0 @@ execute.cpp.o:
-0000000000000000 V typeinfo name for fizzy::instantiate_error
@@ -53 +67,0 @@ execute.cpp.o:
-0000000000000000 V vtable for fizzy::instantiate_error
@@ -66,3 +79,0 @@ parser.cpp.o:
-0000000000000000 W fizzy::parser_error::~parser_error()
-0000000000000000 W fizzy::parser_error::~parser_error()
-0000000000000000 W fizzy::parser_error::~parser_error()
@@ -73,3 +83,0 @@ parser.cpp.o:
-0000000000000000 W fizzy::validation_error::~validation_error()
-0000000000000000 W fizzy::validation_error::~validation_error()
-0000000000000000 W fizzy::validation_error::~validation_error()
@@ -129,6 +136,0 @@ parser.cpp.o:
-0000000000000000 V typeinfo for fizzy::parser_error
-0000000000000000 V typeinfo for fizzy::validation_error
-0000000000000000 V typeinfo name for fizzy::parser_error
-0000000000000000 V typeinfo name for fizzy::validation_error
-0000000000000000 V vtable for fizzy::parser_error
-0000000000000000 V vtable for fizzy::validation_error
@@ -141,3 +142,0 @@ parser_expr.cpp.o:
-0000000000000000 W fizzy::parser_error::~parser_error()
-0000000000000000 W fizzy::parser_error::~parser_error()
-0000000000000000 W fizzy::parser_error::~parser_error()
@@ -145,3 +143,0 @@ parser_expr.cpp.o:
-0000000000000000 W fizzy::validation_error::~validation_error()
-0000000000000000 W fizzy::validation_error::~validation_error()
-0000000000000000 W fizzy::validation_error::~validation_error()
@@ -153,6 +148,0 @@ parser_expr.cpp.o:
-0000000000000000 V typeinfo for fizzy::parser_error
-0000000000000000 V typeinfo for fizzy::validation_error
-0000000000000000 V typeinfo name for fizzy::parser_error
-0000000000000000 V typeinfo name for fizzy::validation_error
-0000000000000000 V vtable for fizzy::parser_error
-0000000000000000 V vtable for fizzy::validation_error

@chfast chfast requested review from axic and gumb0 September 4, 2020 19:03
@codecov
Copy link

codecov bot commented Sep 4, 2020

Codecov Report

Merging #521 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #521   +/-   ##
=======================================
  Coverage   99.69%   99.69%           
=======================================
  Files          54       54           
  Lines       17322    17325    +3     
=======================================
+ Hits        17270    17273    +3     
  Misses         52       52           

@axic
Copy link
Member

axic commented Sep 4, 2020

Does this potentially have any effect on benchmarks?

@chfast
Copy link
Collaborator Author

chfast commented Sep 4, 2020

Does this potentially have any effect on benchmarks?

Unlikely, as we don't call exception destructors (some runtime library does so it cannot inline them anyway).

This fixes -Wweak-vtable warnings
Copy link
Member

@axic axic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to see some numbers, I hope there are no weird regressions.

@chfast
Copy link
Collaborator Author

chfast commented Sep 4, 2020

fizzy/execute/blake2b/512_bytes_rounds_1_mean                     -0.0164         -0.0164            85            83            85            83
fizzy/execute/blake2b/512_bytes_rounds_16_mean                    -0.0165         -0.0165          1279          1258          1279          1258
fizzy/execute/ecpairing/onepoint_mean                             +0.0016         +0.0016        420841        421501        420847        421505
fizzy/execute/keccak256/512_bytes_rounds_1_mean                   -0.0434         -0.0434           108           103           108           103
fizzy/execute/keccak256/512_bytes_rounds_16_mean                  -0.0297         -0.0297          1559          1513          1559          1513
fizzy/execute/memset/256_bytes_mean                               +0.0013         +0.0013             7             7             7             7
fizzy/execute/memset/60000_bytes_mean                             -0.0000         -0.0000          1543          1542          1543          1542
fizzy/execute/mul256_opt0/input0_mean                             -0.0013         -0.0013            26            26            26            26
fizzy/execute/mul256_opt0/input1_mean                             -0.0004         -0.0004            26            26            26            26
fizzy/execute/ramanujan_pi/33_runs_mean                           -0.0218         -0.0218           127           124           127           124
fizzy/execute/sha1/512_bytes_rounds_1_mean                        +0.0033         +0.0033            90            90            90            90
fizzy/execute/sha1/512_bytes_rounds_16_mean                       +0.0039         +0.0039          1252          1257          1252          1257
fizzy/execute/sha256/512_bytes_rounds_1_mean                      -0.0130         -0.0130            93            92            93            92
fizzy/execute/sha256/512_bytes_rounds_16_mean                     -0.0224         -0.0224          1292          1263          1292          1263
fizzy/execute/taylor_pi/pi_1000000_runs_mean                      -0.0010         -0.0010         41401         41359         41402         41359
fizzy/execute/micro/eli_interpreter/halt_mean                     -0.0103         -0.0103             0             0             0             0
fizzy/execute/micro/eli_interpreter/exec105_mean                  +0.0009         +0.0009             5             5             5             5
fizzy/execute/micro/factorial/10_mean                             -0.0043         -0.0043             0             0             0             0
fizzy/execute/micro/factorial/20_mean                             -0.0060         -0.0060             1             1             1             1
fizzy/execute/micro/fibonacci/24_mean                             +0.0041         +0.0041          7785          7817          7785          7817
fizzy/execute/micro/host_adler32/1_mean                           +0.0035         +0.0035             0             0             0             0
fizzy/execute/micro/host_adler32/100_mean                         -0.0108         -0.0108             3             3             3             3
fizzy/execute/micro/host_adler32/1000_mean                        -0.0044         -0.0044            29            29            29            29
fizzy/execute/micro/spinner/1_mean                                +0.0101         +0.0101             0             0             0             0
fizzy/execute/micro/spinner/1000_mean                             -0.0003         -0.0003            10            10            10            10

@chfast
Copy link
Collaborator Author

chfast commented Sep 4, 2020

The size of execute() is unchanged, but fizzy-bench got smaller by 64 bytes :)

@chfast chfast merged commit 1e55c14 into master Sep 4, 2020
@chfast chfast deleted the exception_vtables branch September 4, 2020 21:20
@gumb0 gumb0 mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants