[Debugger Visualizers] Optimize lookup behavior

[Walnut356]

Background

Almost all of the commands in lldb_commands used a regex to associate a type with the synthetic_lookup and summary_lookup python functions. When looking up a type, LLDB iterates through the commands in reverse order (so that new commands can overwrite old ones), stopping when it finds a match. These lookups are cached, but it's a shallow cache (e.g. when Vec<T> is matched by lldb, it will always point to synthetic_lookup, NOT the result of synthetic_lookup which would be StdVecSyntheticProvider).

This becomes a problem because within synthetic_lookup and summary_lookup we run classify_rust_type which checks exact same regexes again. This causes 2 issues:

1. running the regexes via lldb commands is even more of a waste because the final check is a .* regex that associates with synthetic_lookup anyway
2. Every time lldb wants to display a value, that value must run the entirety of synthetic_lookup and run its type through 19 regexes + some assorted checks every single time. Those checks take between 1 and 100 microseconds depending on the type.

On a 10,000 element Vec<i32> (which bypasses classify_struct and therefore the 19 regexes), ~30 milliseconds are spent on classify_rust_type. For a 10,000 element Vec<UserDefinedStruct> that jumps up to ~350 milliseconds.

The salt on the wound is that some of those 19 regexes are useless (BTreeMap and BTreeSet which don't even have synthetic/summary providers so it doesn't matter if we know what type it is), and then the results of that lookup function use string-comparisons in a giant if...elif...elif chain.

Solution

To fix all of that, the lldb_commands now point directly to their appropriate synthetic/summary when possible. In cases where there was extra logic, streamlined functions have been added that have much fewer types being passed in, thus only need to do one or two simple checks (e.g. classify_hashmap and classify_hashset).

Some of the lldb_commands regexes were also consolidated to reduce the total number of commands we pass to lldb (e.g. NonZero

An extra upshot is that summary_lookup could be completely removed due to being redundant.

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Zalathar]

While I'm not comfortable reviewing this myself (and it might be tricky to find someone who is), I do want to at least say thanks for working on it.

[Walnut356]

If it helps the review any, this is more or less how it should have been written from the start. The type synthetic add -l technically expects a python class (-l is short for --python-class), it's sortof a hack that we exploit LLDB not being able to differentiate between an initializer function and a flat function. It's nice because we can check extra bits of the value without callback-based type matching (which was only introduced in lldb 19.0), but I'm not sure why it was written the way it was in cases like Vec<T> that are unambiguous.

The MSVC providers I added a while ago already point directly to the initializer rather than taking a trip through synthetic_lookup and summary_lookup first. This patch just applies that to all of the commands because it's significantly faster.

There shouldn't be any actual changes for end-users (aside from IndirectionSyntheticProvider fixing a minor bug with pointer types).

[Mark-Simulacrum]

r=me if this is still ready to go (not sure if other changes have landed in the last month that make a rebase make sense). I think we can land this and revert/revisit if it runs into issues, the description makes sense to me and the changes seem reasonable.

[jieyouxu]

Changes also look sensible to me. Given that at worst we can revert, let's merge this.

View changes since this review

[jieyouxu]

@bors r=Mark-Simulacrum,jieyouxu rollup

[bors]

📌 Commit 2e8e618 has been approved by Mark-Simulacrum,jieyouxu

It is now in the queue for this repository.

[jieyouxu]

Uh actually @Walnut356, this doesn't need a rebase or anything, right? If not, please r= us
@bors r-
@bors delegate+

[bors]

✌️ @Walnut356, you can now approve this pull request!

If @jieyouxu told you to "r=me" after making some further change, please make that change, then do @bors r=@jieyouxu

[Walnut356]

@bors r=@jieyouxu

[bors]

📌 Commit 2e8e618 has been approved by jieyouxu

It is now in the queue for this repository.

[jieyouxu]

@bors r=Mark-Simulacrum,jieyouxu

[bors]

💡 This pull request was already approved, no need to approve it again.

[bors]

📌 Commit 2e8e618 has been approved by Mark-Simulacrum,jieyouxu

It is now in the queue for this repository.

[bors]

⌛ Testing commit 2e8e618 with merge 3d00472...

[bors]

💔 Test failed - checks-actions

[Walnut356]

That is_msvc check needed to be changed anyway because SBProcess.GetTriple doesn't work quite how we need it to, but it's pretty crazy that it can return None lol

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Zalathar]

@bors try jobs=aarch64-apple

[rust-bors]

☀️ Try build successful (CI)
Build commit: c4dc492 (c4dc4926c5197ff47b02db345958f28abcc6afe6, parent: 733108b6d4acaa93fe26ae281ea305aacd6aac4e)

[bors]

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