Skip to content

Avoid cloning Name when looking up function and class types#14092

Merged
MichaReiser merged 1 commit intomainfrom
micha/lookup-name
Nov 4, 2024
Merged

Avoid cloning Name when looking up function and class types#14092
MichaReiser merged 1 commit intomainfrom
micha/lookup-name

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Nov 4, 2024

Summary

Uses Salsa's new Lookup functionality for FunctionType and ClassType names.
The Lookup trait allows looking-up interned values by a borrowed value. In this case
we can lookup the interned type using the borrowed str and only allocate the owned `Name`` when
the type doesn't already exist.

This was part of #14087 but I extracted it into its own PR to see if it is the reason for the significant incremental perf improvement.

Test Plan

cargo test

@MichaReiser MichaReiser added internal An internal refactor or improvement ty Multi-file analysis & type inference labels Nov 4, 2024
@MichaReiser MichaReiser merged commit bc0586d into main Nov 4, 2024
@MichaReiser MichaReiser deleted the micha/lookup-name branch November 4, 2024 14:53
@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@carljm
Copy link
Contributor

carljm commented Nov 4, 2024

This was part of #14087 but I extracted it into its own PR to see if it is the reason for the significant incremental perf improvement.

Was it? I don't see any CodSpeed results reported here.

@MichaReiser
Copy link
Member Author

Nope, this wasn't it :)

carljm added a commit to Glyphack/ruff that referenced this pull request Nov 5, 2024
* main: (39 commits)
  Also remove trailing comma while fixing C409 and C419 (astral-sh#14097)
  Re-enable clippy `useless-format` (astral-sh#14095)
  Derive message formats macro support to string (astral-sh#14093)
  Avoid cloning `Name` when looking up function and class types (astral-sh#14092)
  Replace `format!` without parameters with `.to_string()` (astral-sh#14090)
  [red-knot] Do not panic when encountering string annotations (astral-sh#14091)
  [red-knot] Add MRO resolution for classes (astral-sh#14027)
  [red-knot] Remove `Type::None` (astral-sh#14024)
  Cached inference of all definitions in an unpacking (astral-sh#13979)
  Update dependency uuid to v11 (astral-sh#14084)
  Update Rust crate notify to v7 (astral-sh#14083)
  Update cloudflare/wrangler-action action to v3.11.0 (astral-sh#14080)
  Update dependency mdformat-mkdocs to v3.1.1 (astral-sh#14081)
  Update pre-commit dependencies (astral-sh#14082)
  Update dependency ruff to v0.7.2 (astral-sh#14077)
  Update NPM Development dependencies (astral-sh#14078)
  Update Rust crate thiserror to v1.0.67 (astral-sh#14076)
  Update Rust crate syn to v2.0.87 (astral-sh#14075)
  Update Rust crate serde to v1.0.214 (astral-sh#14074)
  Update Rust crate pep440_rs to v0.7.2 (astral-sh#14073)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal An internal refactor or improvement ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants