Skip to content

Use cmx reachability to decide locality of symbols#1246

Merged
mshinwell merged 6 commits intooxcaml:mainfrom
Gbury:fewer-symbols
Apr 26, 2023
Merged

Use cmx reachability to decide locality of symbols#1246
mshinwell merged 6 commits intooxcaml:mainfrom
Gbury:fewer-symbols

Conversation

@Gbury
Copy link
Copy Markdown
Contributor

@Gbury Gbury commented Mar 22, 2023

This builds on top of #1222 .

The first commit adds some useful printing functions, as well as prefix all symbols in the cmm output to indicate whether the symbol is global or local (which is extremely helpful for debugging, as most of the bugs I've come across were because the same symbol was declared and used with different "localities").

The second commit centralizes the handling of symbols in to_cmm. Basically, after this commit, all cmm symbols are now generated in to_cmm_result.ml, and there should not be anywhere that still uses strings for symbols (instead using either an flambda Symbol.t or a Cmm.symbol created by the appropriate function in to_cmm_result.ml).

Finally, the third commit uses the name occurrences that are computed during cmx file preparation to decide which symbols should be local. The exact criterion is to use local cmm symbols for symbols from the current compilation unit, and that do not appear in the reachable names.

@mshinwell mshinwell added flambda2 Prerequisite for, or part of, flambda2 compilation speed Potential compilation speed improvements labels Mar 22, 2023
@stedolan
Copy link
Copy Markdown
Contributor

stedolan commented Mar 22, 2023

Sounds good!

I haven't read the code in detail, but from the description I wonder whether there's a missed optimisation: it is in fact possible and useful to have a Local reference to a Global symbol. (This occurs when a symbol is exported, but there are also references to that symbol from within the current file).

To see how much impact it's having, I recommend running these commands before and after the change:

$ nm _install/lib/ocaml/compiler-libs/ocamlcommon.a  | wc -l
$ objdump -r _install/lib/ocaml/compiler-libs/ocamlcommon.a  | wc -l
$ objdump -r _install/lib/ocaml/compiler-libs/ocamlcommon.a  | grep -v ' \.' | wc -l

These are (approximately, wc -l includes headers etc) the counts of:

  • symbols
  • relocations
  • relocations with symbol references (as opposed to section references)

I'd expect all three to decrease. Also, it's better to measure after configuring with --disable-function-sections.

@Gbury
Copy link
Copy Markdown
Contributor Author

Gbury commented Mar 22, 2023

Yeah, I was reviewing #1222 and saw that indeed we could do that. I'll try and see what happens.

@Gbury Gbury mentioned this pull request Mar 22, 2023
Merged
@Gbury
Copy link
Copy Markdown
Contributor Author

Gbury commented Mar 22, 2023

I implemented the use of Local for symbols defined in the same compilation unit (see the branch even_fewer_symbols on my fork).

Here are the results of the commands (configured with --disable-function-section):

main this PR even_fewer_symbols
nm 49238 29221 (-40%) 29218 (-40%)
objdump -r 226643 198975 (-12%) 194792 (-14%)
objdump -r | grep -v ' \.' 151517 91836 (-40%) 72655 (-52%)
ocamlopt.opt 49M 44M (-10%) 44M (-10%)

Gbury
Gbury commented Apr 26, 2023
View reviewed changes
@mshinwell mshinwell merged commit 06173dc into oxcaml:main Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mshinwell mshinwell mshinwell approved these changes

@gretay-js gretay-js Awaiting requested review from gretay-js

@lthls lthls Awaiting requested review from lthls

@xclerc xclerc Awaiting requested review from xclerc

Assignees

No one assigned

Labels

compilation speed Potential compilation speed improvements flambda2 Prerequisite for, or part of, flambda2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Gbury @stedolan @mshinwell