[PATCH] D129712: [WIP] [Bitstream] take Expected<T> off critical reading paths

[Sam McCall via Phabricator via llvm-commits]

sammccall added a comment.

This is very much a prototype, hopefully the idea is clear.

Would appreciate some feedback on:

- is this a reasonable/comprehensible error-handling scheme, does it introduce too much tech debt for the performance win etc
- whether to keep the E versions of the functions private, whether to make both versions public, or whether to completely replace the old versions
- better naming

Profile before
F23786310: image.png <https://reviews.llvm.org/F23786310>
Profile after
F23786313: image.png <https://reviews.llvm.org/F23786313>

The workload is clangd --check=AST.cpp, modified to rebuild the AST 1000 times, it's roughly 2/3rds buildAST.
There's about 4% savings clearly visible, and 4% divided by 2/3rs is close to the 7% speedup I saw in the logs.

I would expect this to generalize to other PCH/modules use.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129712/new/

https://reviews.llvm.org/D129712