Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[EXPERIMENT] Determine cause of perf regression in #108250 #108327

Closed
wants to merge 5 commits into from
Closed

[EXPERIMENT] Determine cause of perf regression in #108250 #108327

wants to merge 5 commits into from

Commits on Feb 22, 2023

  1. Tweak the slice interners. 

    All the slice interners have a wrapper that handles the empty slice
case. We can instead handle this in the `slice_interners!` macro,
avoiding the need for most of the wrappers, and allowing the interner
functions to be renamed from `_intern_foos` to `intern_foos`.

The two exceptions:
- intern_predicates: I kept this wrapper because there's a FIXME
  comment about a possible future change.
- intern_poly_existential_predicates: I kept this wrapper because it
  asserts that the slice is empty and sorted.
    @nnethercote
    nnethercote committed Feb 22, 2023
    Configuration menu
    Copy the full SHA
    1cffe2a View commit details
    Browse the repository at this point in the history

  2. Rename many interner functions. 

    (This is a large commit. The changes to
`compiler/rustc_middle/src/ty/context.rs` are the most important ones.)

The current naming scheme is a mess, with a mix of `_intern_`, `intern_`
and `mk_` prefixes, with little consistency. In particular, in many
cases it's easy to use an iterator interner when a (preferable) slice
interner is available.

The guiding principles of the new naming system:
- No `_intern_` prefixes.
- The `intern_` prefix is for internal operations.
- The `mk_` prefix is for external operations.
- For cases where there is a slice interner and an iterator interner,
  the former is `mk_foo` and the latter is `mk_foo_from_iter`.

Also, `slice_interners!` and `direct_interners!` can now be `pub` or
non-`pub`, which helps enforce the internal/external operations
division.

It's not perfect, but I think it's a clear improvement.

The following lists show everything that was renamed.

slice_interners
- const_list
  - mk_const_list -> mk_const_list_from_iter
  - intern_const_list -> mk_const_list
- substs
  - mk_substs -> mk_substs_from_iter
  - intern_substs -> mk_substs
  - check_substs -> check_and_mk_substs (this is a weird one)
- canonical_var_infos
  - intern_canonical_var_infos -> mk_canonical_var_infos
- poly_existential_predicates
  - mk_poly_existential_predicates -> mk_poly_existential_predicates_from_iter
  - intern_poly_existential_predicates -> mk_poly_existential_predicates
  - _intern_poly_existential_predicates -> intern_poly_existential_predicates
- predicates
  - mk_predicates -> mk_predicates_from_iter
  - intern_predicates -> mk_predicates
  - _intern_predicates -> intern_predicates
- projs
  - intern_projs -> mk_projs
- place_elems
  - mk_place_elems -> mk_place_elems_from_iter
  - intern_place_elems -> mk_place_elems
- bound_variable_kinds
  - mk_bound_variable_kinds -> mk_bound_variable_kinds_from_iter
  - intern_bound_variable_kinds -> mk_bound_variable_kinds

direct_interners
- region
  - intern_region (unchanged)
- const
  - mk_const_internal -> intern_const
- const_allocation
  - intern_const_alloc -> mk_const_alloc
- layout
  - intern_layout -> mk_layout
- adt_def
  - intern_adt_def -> mk_adt_def_from_data (unusual case, hard to avoid)
  - alloc_adt_def(!) -> mk_adt_def
- external_constraints
  - intern_external_constraints -> mk_external_constraints

Other
- type_list
  - mk_type_list -> mk_type_list_from_iter
  - intern_type_list -> mk_type_list
- tup
  - mk_tup -> mk_tup_from_iter
  - intern_tup -> mk_tup
    @nnethercote
    nnethercote committed Feb 22, 2023
    Configuration menu
    Copy the full SHA
    1ee8024 View commit details
    Browse the repository at this point in the history

  3. Replace a mk_ty call with mk_bound.

    @nnethercote
    nnethercote committed Feb 22, 2023
    Configuration menu
    Copy the full SHA
    c09ebf9 View commit details
    Browse the repository at this point in the history

  4. Rename mk_{ty,region} as mk_{ty,region}_from_kind. 

    To discourage accidental use -- there are more specific `mk_*` functions
for all `Ty` and `Region` kinds.
    @nnethercote
    nnethercote committed Feb 22, 2023
    Configuration menu
    Copy the full SHA
    f63847d View commit details
    Browse the repository at this point in the history

  5. Add mk_canonical_var_infos_from_iter. 

    It's missing, and is useful in two places.
    @nnethercote
    nnethercote committed Feb 22, 2023
    Configuration menu
    Copy the full SHA
    254d119 View commit details
    Browse the repository at this point in the history