Linking: perform macro expansion and type updates just once #2254

tautschnig · 2018-05-29T15:23:59Z

This reduces the Linux kernel link time by more than 90% as the previous
approach would repeatedly attempt renamings and replacements across all
hitherto read goto program instructions.

Depends on:

C++ front end fixes [blocks: #2554] #1260

smowton

Minor comments, looks good. Can you think of any corner cases to test aren't broken, e.g. unifying two units with a weak defn vs. one with a strong defn, or similarly two with incomplete definitions of a struct vs. one that provides the definition? If so are you happy we've already got regression tests taking care of those?

smowton · 2018-05-29T16:08:33Z

src/goto-cc/compile.cpp

-  }
+  goto_modelt goto_model;
+
+  goto_model.symbol_table.swap(symbol_table);


Surely just introduce a <symbol-table, goto-functions> overload -- or perhaps use wrapper_goto_modelt?

Hmm, none of wrapper_goto_modelt nor abstract_goto_modelt provide non-const accessors. Is there are good reason those should not be added? (I strictly need non-const access here.)

abstract_goto_modelt should not provide a non-const accessor (as lazy-goto-model must enforce that you only use its proper interface to populate new functions, for example). However wrapper_goto_modelt and goto_modelt may reasonably implement something stronger with non-const accessors.

smowton · 2018-05-29T16:09:33Z

src/goto-programs/link_goto_model.cpp

+      weak_symbols.insert(symbol_pair.first);
+  }
+
+  linkingt linking(dest.symbol_table,


Style (break after open-paren)

smowton · 2018-05-29T16:11:40Z

src/goto-programs/initialize_goto_model.cpp

-  std::vector<std::string> binaries, sources;
-  binaries.reserve(files.size());
-  sources.reserve(files.size());
+  std::list<std::string> binaries, sources;


std::list is quite surprising -- do we really need any of list's properties (cheap insert / delete, stable iterators)? If not I'd stick with vector

I'd prefer to say "use whatever SequenceContainer you want," but then I can't do that unless I add a template parameter. The difference will not be measurable. The number of entries will typically be a very small number, with the 1000-2000 entries that the Linux kernel yields being close to an upper bound. I have picked std::list, because the number of entries is not known a priori in compilet, and hence append-at-the-end would be inefficient with std::vector. Again, "inefficient" in theory, practically not measurably so.

ok, not a big deal either way

I'm not going to insist on using std::list either - but I did have to change one or the other, and if you want me to flip a coin I can do that as well :-)

tautschnig · 2018-05-29T18:01:56Z

Just a note: it seems I cannot further postpone fixing #1490, which is what the CI failures are.

tautschnig · 2018-05-30T15:03:18Z

I am now including some commits from #1260.

tautschnig · 2018-06-01T10:35:13Z

I am working on further fixes in #1260, which should then get merged before this one.

src/goto-programs/read_goto_binary.cpp

src/symtab2gb/symtab2gb_parse_options.cpp

TGWDB · 2021-07-09T09:36:10Z

@tautschnig This PR looks like it is close being merged, apart from codeowner review, is there anything outstanding here?

martin-cs · 2021-07-09T10:59:08Z

Also paging @chrisr-diffblue and @peterschrammel

chrisr-diffblue

That looks like a sensible improvement :-)

tautschnig · 2021-08-03T18:19:16Z

@tautschnig This PR looks like it is close being merged, apart from codeowner review, is there anything outstanding here?

I had to do a rebase to resolve conflicts, but now it's really just codeowner reviews that are missing.

There is no need to do the additional work that linking on goto functions entails when the input is just symbol tables. Thus link symbol tables, and only after linking all of them generate goto functions and then write the combined goto model.

We do the same work in multiple places, and can optimize the linking process when knowing about all files to be linked in one step.

This reduces the Linux kernel link time by more than 90% as the previous approach would repeatedly attempt renamings and replacements across all hitherto read goto program instructions.

tautschnig requested review from kroening, peterschrammel and smowton as code owners May 29, 2018 15:24

tautschnig self-assigned this May 29, 2018

smowton approved these changes May 29, 2018

View reviewed changes

tautschnig force-pushed the linear-linking branch from 865e875 to 2a21bb5 Compare May 30, 2018 14:54

tautschnig requested review from forejtv and thk123 as code owners May 30, 2018 14:54

tautschnig force-pushed the linear-linking branch from 2a21bb5 to e28b874 Compare May 30, 2018 15:00

tautschnig assigned kroening and peterschrammel and unassigned kroening and peterschrammel May 30, 2018

tautschnig force-pushed the linear-linking branch from e28b874 to 92be095 Compare January 31, 2019 13:45

tautschnig requested a review from pkesseli as a code owner January 31, 2019 13:45

tautschnig changed the title ~~Linking: perform macro expansion and type updates just once~~ Linking: perform macro expansion and type updates just once [depends-on: #1260] Feb 1, 2019

tautschnig added the dependent - do not merge label Feb 1, 2019

tautschnig changed the title ~~Linking: perform macro expansion and type updates just once [depends-on: #1260]~~ Linking: perform macro expansion and type updates just once May 22, 2019

tautschnig force-pushed the linear-linking branch from 92be095 to 7eedf66 Compare May 22, 2019 07:25

tautschnig added Performance Optimisations and removed dependent - do not merge labels May 22, 2019

tautschnig force-pushed the linear-linking branch from 7eedf66 to f1671af Compare May 22, 2019 07:28

tautschnig requested a review from chrisr-diffblue as a code owner May 22, 2019 07:28

tautschnig assigned kroening and unassigned tautschnig May 22, 2019

tautschnig force-pushed the linear-linking branch 2 times, most recently from e0288d1 to e868217 Compare May 27, 2019 18:24

tautschnig requested a review from romainbrenguier as a code owner January 28, 2021 17:13

tautschnig force-pushed the linear-linking branch from 0635c92 to 4ea5de8 Compare January 28, 2021 17:17

martin-cs approved these changes Jan 28, 2021

View reviewed changes

src/goto-programs/read_goto_binary.cpp Outdated Show resolved Hide resolved

src/goto-programs/read_goto_binary.cpp Outdated Show resolved Hide resolved

src/symtab2gb/symtab2gb_parse_options.cpp Show resolved Hide resolved

tautschnig force-pushed the linear-linking branch from 4ea5de8 to 21a9ef9 Compare January 29, 2021 19:56

tautschnig assigned kroening and unassigned tautschnig Jan 29, 2021

tautschnig force-pushed the linear-linking branch from 21a9ef9 to 2ac5414 Compare February 1, 2021 20:25

tautschnig force-pushed the linear-linking branch 2 times, most recently from 0667ded to 1249602 Compare February 12, 2021 10:14

tautschnig force-pushed the linear-linking branch 4 times, most recently from a7778b0 to 5f4d522 Compare March 17, 2021 00:18

tautschnig force-pushed the linear-linking branch 2 times, most recently from 45fb1b7 to e77cebb Compare May 7, 2021 14:19

chrisr-diffblue approved these changes Jul 9, 2021

View reviewed changes

tautschnig force-pushed the linear-linking branch from e77cebb to 353b12d Compare August 3, 2021 18:18

tautschnig force-pushed the linear-linking branch from 353b12d to 4995057 Compare December 5, 2021 23:57

tautschnig added 3 commits December 26, 2021 15:33

Encapsulate reading and linking multiple files

895a674

We do the same work in multiple places, and can optimize the linking process when knowing about all files to be linked in one step.

Linking: perform macro expansion and type updates just once

ee42ac6

This reduces the Linux kernel link time by more than 90% as the previous approach would repeatedly attempt renamings and replacements across all hitherto read goto program instructions.

tautschnig force-pushed the linear-linking branch from 4995057 to ee42ac6 Compare December 27, 2021 09:22

tautschnig assigned peterschrammel and unassigned kroening Dec 27, 2021

peterschrammel approved these changes Dec 27, 2021

View reviewed changes

tautschnig merged commit 8c4d15b into diffblue:develop Dec 27, 2021

tautschnig deleted the linear-linking branch December 27, 2021 11:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linking: perform macro expansion and type updates just once #2254

Linking: perform macro expansion and type updates just once #2254

tautschnig commented May 29, 2018 •

edited

Loading

smowton left a comment

smowton May 29, 2018

tautschnig May 29, 2018

smowton May 29, 2018

smowton May 29, 2018

smowton May 29, 2018

tautschnig May 29, 2018 •

edited

Loading

smowton May 29, 2018

tautschnig May 29, 2018

tautschnig commented May 29, 2018

tautschnig commented May 30, 2018

tautschnig commented Jun 1, 2018

TGWDB commented Jul 9, 2021

martin-cs commented Jul 9, 2021

chrisr-diffblue left a comment

tautschnig commented Aug 3, 2021

Linking: perform macro expansion and type updates just once #2254

Linking: perform macro expansion and type updates just once #2254

Conversation

tautschnig commented May 29, 2018 • edited Loading

smowton left a comment

Choose a reason for hiding this comment

smowton May 29, 2018

Choose a reason for hiding this comment

tautschnig May 29, 2018

Choose a reason for hiding this comment

smowton May 29, 2018

Choose a reason for hiding this comment

smowton May 29, 2018

Choose a reason for hiding this comment

smowton May 29, 2018

Choose a reason for hiding this comment

tautschnig May 29, 2018 • edited Loading

Choose a reason for hiding this comment

smowton May 29, 2018

Choose a reason for hiding this comment

tautschnig May 29, 2018

Choose a reason for hiding this comment

tautschnig commented May 29, 2018

tautschnig commented May 30, 2018

tautschnig commented Jun 1, 2018

TGWDB commented Jul 9, 2021

martin-cs commented Jul 9, 2021

chrisr-diffblue left a comment

Choose a reason for hiding this comment

tautschnig commented Aug 3, 2021

tautschnig commented May 29, 2018 •

edited

Loading

tautschnig May 29, 2018 •

edited

Loading