Consider using unreachable_unchecked in hot spots

[ljedrz]

hint::unreachable_unchecked is nearly twice as fast as its safe counterpart (benchmarks for 1000 matches):

test bench_unreachable_safe   ... bench:         687 ns/iter (+/- 30)
test bench_unreachable_unsafe ... bench:         346 ns/iter (+/- 11)

It might be a good idea to use it in hot 🔥 spots where we are super sure™️ we won't ever reach it or with debug_asserts right before it.

I have ripgrepped (ar-geed?) for unreachable!()s in the compiler (list below) but I don't know which ones are hot enough. If this is a good idea I'd be happy to change them if anyone can advise where this should be considered.

unreachable!()s

• liballoc\borrow.rs:211
• liballoc\raw_vec.rs:424
• liballoc\raw_vec.rs:504
• libpanic_unwind\emcc.rs:49
• libproc_macro\quote.rs:131
• libproc_macro\rustc.rs:182
• libproc_macro\rustc.rs:183
• libproc_macro\rustc.rs:263
• librustc_apfloat\ieee.rs:1759
• librustc_codegen_llvm\intrinsic.rs:1292
• librustc_codegen_llvm\intrinsic.rs:1303
• librustc_codegen_llvm\intrinsic.rs:1368
• librustc_codegen_llvm\intrinsic.rs:1465
• librustc_data_structures\small_vec.rs:98
• librustc_driver\lib.rs:254
• librustc_driver\pretty.rs:914
• librustc_driver\pretty.rs:1065
• librustc_driver\pretty.rs:1117
• librustc_driver\pretty.rs:1123
• librustc_driver\pretty.rs:1153
• librustc_errors\lib.rs:751
• librustc_metadata\encoder.rs:868
• librustc_metadata\locator.rs:429
• librustc_plugin\build.rs:64
• librustc_privacy\lib.rs:385
• librustc_resolve\build_reduced_graph.rs:175
• librustc_resolve\build_reduced_graph.rs:469
• librustc_resolve\build_reduced_graph.rs:508
• librustc_resolve\macros.rs:144
• librustc_resolve\macros.rs:167
• librustc_resolve\macros.rs:515
• librustc_resolve\macros.rs:683
• librustc_resolve\macros.rs:848
• librustc_resolve\resolve_imports.rs:202
• librustc_resolve\resolve_imports.rs:307
• librustc_resolve\resolve_imports.rs:584
• librustc_resolve\resolve_imports.rs:749
• librustc_resolve\lib.rs:1599
• librustc_save_analysis\dump_visitor.rs:514
• librustc_typeck\astconv.rs:386
• librustc_typeck\astconv.rs:1389
• librustdoc\fold.rs:37
• libserialize\json.rs:1734
• libstd\macros.rs:301
• libstd\path.rs:956
• libstd\path.rs:1003
• libsyntax_pos\lib.rs:862
• libsyntax_pos\lib.rs:910
• libsyntax\tokenstream.rs:293
• libsyntax\tokenstream.rs:305
• libsyntax\tokenstream.rs:319
• libsyntax\tokenstream.rs:463
• libterm\win.rs:68
• libterm\win.rs:88
• libcore\iter\mod.rs:1180
• libcore\iter\mod.rs:2038
• libcompiler_builtins\src\arm_linux.rs:30
• librustc\hir\def_id.rs:107
• librustc\hir\lowering.rs:3777
• librustc\hir\print.rs:2060
• librustc\hir\print.rs:2227
• librustc\lint\context.rs:305
• librustc\mir\tcx.rs:311
• librustc\middle\reachable.rs:374
• librustc\session\mod.rs:1299
• librustc\traits\project.rs:174
• librustc_borrowck\borrowck\mod.rs:731
• librustc_codegen_llvm\back\lto.rs:200
• librustc_codegen_llvm\mir\rvalue.rs:586
• librustc_codegen_llvm\mir\rvalue.rs:672
• librustc_codegen_llvm\mir\rvalue.rs:766
• librustc_codegen_llvm\mir\rvalue.rs:781
• librustc_codegen_llvm\mir\rvalue.rs:796
• librustc_data_structures\obligation_forest\mod.rs:518
• librustc_data_structures\obligation_forest\mod.rs:537
• librustc_driver\profile\mod.rs:139
• librustc_driver\profile\mod.rs:140
• librustc_driver\profile\mod.rs:307
• librustc_driver\profile\mod.rs:310
• librustc_incremental\persist\dirty_clean.rs:468
• librustc_mir\borrow_check\error_reporting.rs:368
• librustc_mir\borrow_check\mod.rs:1204
• librustc_mir\borrow_check\mod.rs:1753
• librustc_mir\borrow_check\move_errors.rs:246
• librustc_mir\borrow_check\mutability_errors.rs:127
• librustc_mir\borrow_check\mutability_errors.rs:331
• librustc_mir\build\scope.rs:946
• librustc_mir\transform\add_moves_for_packed_drops.rs:115
• librustc_mir\transform\simplify.rs:166
• librustc_typeck\check\coercion.rs:1271
• librustc_typeck\check\compare_method.rs:829
• librustc_typeck\check\compare_method.rs:889
• librustc_typeck\check\mod.rs:1859
• librustc_typeck\check\mod.rs:4984
• librustc_typeck\check\wfcheck.rs:389
• libstd\io\error.rs:215
• libstd\sync\rwlock.rs:227
• libsyntax\attr\builtin.rs:395
• libsyntax\diagnostics\plugin.rs:48
• libsyntax\diagnostics\plugin.rs:93
• libsyntax\diagnostics\plugin.rs:157
• libsyntax\ext\expand.rs:302
• libsyntax\ext\expand.rs:424
• libsyntax\ext\expand.rs:554
• libsyntax\ext\expand.rs:709
• libsyntax\ext\expand.rs:898
• libsyntax\ext\expand.rs:1221
• libsyntax\ext\expand.rs:1304
• libsyntax\ext\expand.rs:1416
• libsyntax\ext\placeholders.rs:103
• libsyntax\print\pprust.rs:2807
• libsyntax\print\pprust.rs:3051
• libsyntax\parse\parser.rs:633
• libsyntax\parse\parser.rs:2168
• libsyntax\parse\parser.rs:2708
• libsyntax\parse\parser.rs:2744
• libsyntax\parse\parser.rs:4419
• libsyntax\parse\parser.rs:4427
• libsyntax\parse\parser.rs:4436
• libsyntax\parse\parser.rs:5263
• libsyntax_ext\deriving\debug.rs:125
• libterm\terminfo\parm.rs:169
• libterm\terminfo\parm.rs:189
• libterm\terminfo\parm.rs:230
• tools\linkchecker\main.rs:249
• liballoc\collections\btree\map.rs:167
• liballoc\collections\btree\map.rs:982
• liballoc\collections\btree\map.rs:1060
• liballoc\collections\btree\map.rs:2511
• liballoc\collections\btree\node.rs:1428
• liballoc\collections\btree\node.rs:1489
• liballoc\collections\btree\node.rs:1574
• libcore\num\dec2flt\rawfp.rs:301
• libcore\num\dec2flt\rawfp.rs:308
• librustc\infer\error_reporting\mod.rs:201
• librustc\ty\query\on_disk_cache.rs:661
• librustc\ty\query\on_disk_cache.rs:754
• librustc\ty\query\plumbing.rs:626
• librustc_mir\borrow_check\nll\invalidation.rs:440
• librustc_target\abi\call\mod.rs:379
• libstd\collections\hash\map.rs:804
• libstd\collections\hash\map.rs:999
• libstd\collections\hash\map.rs:3472
• libstd\collections\hash\map.rs:3484
• libstd\collections\hash\map.rs:3496
• libstd\collections\hash\map.rs:3507
• libstd\collections\hash\table.rs:720
• libstd\collections\hash\table.rs:761
• libstd\sync\mpsc\oneshot.rs:124
• libstd\sync\mpsc\oneshot.rs:190
• libstd\sync\mpsc\oneshot.rs:212
• libstd\sync\mpsc\oneshot.rs:269
• libstd\sync\mpsc\oneshot.rs:298
• libstd\sync\mpsc\oneshot.rs:337
• libstd\sync\mpsc\oneshot.rs:362
• libstd\sync\mpsc\mod.rs:851
• libstd\sync\mpsc\mod.rs:898
• libstd\sync\mpsc\mod.rs:916
• libstd\sync\mpsc\mod.rs:1206
• libstd\sync\mpsc\mod.rs:1214
• libstd\sync\mpsc\mod.rs:1221
• libstd\sync\mpsc\select.rs:386
• libstd\sync\mpsc\shared.rs:337
• libstd\sync\mpsc\spsc_queue.rs:279
• libstd\sync\mpsc\spsc_queue.rs:286
• libstd\sync\mpsc\stream.rs:363
• libstd\sync\mpsc\stream.rs:392
• libstd\sync\mpsc\stream.rs:484
• libstd\sync\mpsc\sync.rs:126
• libstd\sync\mpsc\sync.rs:143
• libstd\sync\mpsc\sync.rs:251
• libstd\sync\mpsc\sync.rs:267
• libstd\sync\mpsc\sync.rs:338
• libstd\sync\mpsc\sync.rs:380
• libstd\sync\mpsc\sync.rs:412
• libstd\sync\mpsc\sync.rs:440
• libstd\sync\mpsc\sync.rs:441
• libstd\sys\unix\fs.rs:507
• libstd\sys\redox\os.rs:207
• libsyntax\ext\tt\macro_parser.rs:755
• libsyntax\ext\tt\macro_rules.rs:196
• libsyntax\parse\lexer\mod.rs:1392
• libsyntax_ext\deriving\generic\mod.rs:468
• libcompiler_builtins\libm\src\math\rem_pio2_large.rs:465
• libstd\sys\redox\syscall\call.rs:21

[oli-obk]

I've been saved by the compiler panicking with "entered unreachable code" quite a few times. Especially during development where one might end up making such code reachable (maybe because a new features was implemented and some other code correctly assumed that it was unreachable before the existence of the new feature). Having these silently become UB seems... suboptimal.

[ljedrz]

That's why I'm suggesting adding debug_assert!()s; the sort of changes you are describing doesn't go straight into optimized builds, so these problems would still be caught and the extra safeguards would not affect the release code.

[ljedrz]

cc @nnethercote

[nnethercote]

To be honest, I wouldn't trust a microbenchmark very much for this kind of change. I think the behaviour in real code could be quite different.

But it might be worth doing a mass change and then measuring to see if it makes a difference in practice. If it does, then it's reasonable to have a discussion about whether the speed up is worth the functionality changes.

[ishitatsuyuki]

It's not worth doing in most cases, because:

• You won't see a difference unless it's very hot.
• You lose the safety guarantees and now need to fear UB.
• LLVM sometimes can eliminate the redundant match arm.

[dtolnay]

I would like to see @nnethercote's suggestion benchmarked -- blindly replace every unreachable with unreachable_unchecked and check the benchmarks to determine an upper bound on how much we stand to gain by intelligently replacing a few hot cases with unchecked.

[ljedrz]

@dtolnay It worked with assertions (#53035) - changing them all to debug_assert and doing a perf run pointed to cases where they were most expensive.

[ljedrz]

A perf run with all the unreachable!()s changed to their faster counterpart didn't show a noticeable difference, so we can close this issue.