[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Hey! Sorry for the late reply, this has been open in my tabs since day 1 just didn't get a chance to take a look at it.

The biggest problem I see is, this is not changing the fact that we are still traversing the whole file:

- You do one traversal over the whole file to find `FileLines`
- Then possibly two more to find `OffsetMin` and `OffsetMax`

You can get rid of the first one by just using `getLocForEndOfFile` if `positionToOffset` fails.
For the latter, you can use `SourceManager::translateLineCol` instead, it uses a cache and cheaper than the linear scan performed by `positionToOffset` in case of multiple calls. The cache is already populated once we issue the first `Cost` call, through `SM.getSpellingLineNumber`

I was reluctant overall as the wins aren't clear. We are still going to traverse the whole file at least once, and readability is hindered but I suppose with the above mentioned two trics, runtime shouldn't be regressed.
Also it is nice to get rid of 2 log calls for each token. Again all of these feels like some micro optimizations that aren't justified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891