Enable codegen_units in dev builds

[larsbergstrom]

r? @metajack

This re-enables the use of codegen-units on dev builds, which should be fine because dev's perf is already atrocious anyway. The difference on my i7 iMac:
full build goes from 6m 30s to 6m
rebuild of just the script crate goes from 3m25s to 2m45s

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #__ (github issue number if applicable).

• There are tests for these changes OR
• These changes do not require tests because _____

---

This change is

[metajack]

@bors-servo r+

Do you have numbers for how much 4 units affects performance?

[bors-servo]

📌 Commit 5436c3e has been approved by metajack

[larsbergstrom]

I wasn't able to tell any difference in the performance of the dev build with and without codegen, but it's pretty bad to start with :-)

[bors-servo]

⌛ Testing commit 5436c3e with merge 07772b4...

[bors-servo]

💔 Test failed - android

[larsbergstrom]

@alexcrichton @brson Would you expect that using codegen-units would cause extra symbols to appear in a resulting binary? Because that's what we're seeing on the android cross dev build here:

Unexpected dynamic symbols in binary:
eglCreatePlatformPixmapSurface
eglClientWaitSync
eglGetPlatformDisplay
eglCreatePlatformPixmapSurfaceEXT
eglCreatePlatformWindowSurface
eglCreateSync
eglGetPlatformDisplayEXT
eglDestroyImage
eglGetSyncAttrib
eglDestroySync
eglCreatePlatformWindowSurfaceEXT
eglCreateImage
eglWaitSync
program finished with exit code 255
elapsedTime=0.104437

[alexcrichton]

@larsbergstrom I'd expect more Rust symbols to show up, but not symbols like that... Also the other rust symbols should in theory just be impl details. Weird!

[brson]

I have one theory for the extra native symbols. With more codegen units there are fewer inlining opportunities, fewer interprocedural optimizations, so some opportunities to completely eliminate unused Rust functions may have been lost. Those functions may pull in other non-Rust symbols that the linker can't remove.

[larsbergstrom]

@brson @alexcrichton Aha, thanks, that makes sense!

On other platforms, this isn't as big of a deal, but specifically on Android the loader resolves all external symbols for the transitive dependency graph immediately upon load of a dylib. So, I may just have to switch this cargo config to be only for dev builds that are not Android (where we check for unanticipated symbol references).

[metajack]

Can we not fix the bad linking? It sounds like this may just be exposing errors in our link setup? Sounds like under @brson's theory we are not linking libs which we have code that calls because they got removed due to inlining before.

[larsbergstrom]

We have a lot of problems with this, in general. Basically, we have FFI crates that wrap all the C functions from underlying libraries. We're then at the mercy of the linker toolchain to "notice" that those Rust wrappers are never called or that the Rust code that calls those Rust wrappers is never called and then discard them.

I'm open to other linker flags we could be passing, but this has been the bane of my Android linking existence for a long time (and is why we have the script to check for new symbols, because it's so easy to mess up, since linkers are super fragile here).

[brson]

Another thought. If you are doing regression testing based on the symbol tables, and codegen-units indeed affects the symbols, then you may find yourself running into spurious errors if the mechanism for selecting codegen units is non-deterministic (as it more-or-less was at one time). It's possible that with incremental compilation changes the codegen units are picked deterministically though. @michaelwoerister would know.

[michaelwoerister]

I don't think that there is anything non-deterministic in codegen unit selection. At some point, due to a bug, symbol names for monomorphizations where non-deterministic but that has been fixed. So I'm pretty sure that compiling the same codebase twice will result in the same symbols showing up in the same codegen units.

[larsbergstrom]

@bors-servo try

[larsbergstrom]

@bors-servo r- try clean force

[bors-servo]

☔ The latest upstream changes (presumably #14413) made this pull request unmergeable. Please resolve the merge conflicts.