Closed SimonSapin closed 4 years ago
@bors-servo r+
:pushpin: Commit dbb9bee has been approved by jdm
:hourglass: Testing commit dbb9bee06e0b0168ccae0619c5077e302669d2fb with merge c66102ec319158797b027e831d695e83b1921b17...
@bors-servo r- Sorry, jumped the gun.
@bors-servo r+
:pushpin: Commit dbb9bee has been approved by jdm
:hourglass: Testing commit dbb9bee06e0b0168ccae0619c5077e302669d2fb with merge 28248e1d6658e92dd5ecb0866e53a97f043b9b38...
:broken_heart: Test failed - checks-travis
I'm restarting the single job that timed out.
:sunny: Test successful - checks-travis, status-appveyor Approved by: jdm Pushing 28248e1d6658e92dd5ecb0866e53a97f043b9b38 to master...
Servo has generated code that calls it with thousands of different closures. With monomorphization, this function alone contributed significantly to code size (millions of lines of unoptimized LLVM IR) and compilation time: https://github.com/servo/servo/issues/26585
By switching to dynamic dispatch, compilation time for Servo’s script crate (which is pathologically long) is reduced by about a quarter on my machine.
Other changes are:
Removing return value handling, which required a generic type parameter. Instead, the provided callback needs to assign to a captured variable. (See test for an example.)
The callback needs to implement
FnMut
rather than onlyFnOnce
. We could instead takeBox<dyn FnOnce() + '_>
(with allocation overhead), but none of the callers is affected.The
UnwindSafe
constraint is removed, because multi-trait objects do not support up-casting yet. It wasn’t much help anyway since the code generator always usedAssertUnwindSafe
regardless of what each closure does.Dynamic dispatch has non-zero runtime cost. Perhaps here it is small enough, or compensated by better instruction cache utilization thanks to reduced code size?