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.

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

Adds -Z mir-stats, which is similar to -Z hir-stats. #38092

Merged
merged 1 commit into from
Dec 5, 2016

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Nov 30, 2016

Adds -Z mir-stats, which is similar to -Z hir-stats.

Some notes:

  • This code attempts to present the breakdown of each variant for
    every enum in the MIR. This is meant to guide decisions about how to
    revise representations e.g. when to box payloads for rare variants
    to shrink the size of the enum overall.

  • I left out the "Total:" line that hir-stats presents, because this
    implementation uses the MIR Visitor infrastructure, and the memory
    usage of structures directly embedded in other structures (e.g. the
    func: Operand in a TerminatorKind:Call) is not distinguished
    from similar structures allocated in a Vec (e.g. the args: Vec<Operand> in a TerminatorKind::Call). This means that a naive
    summation of all the accumulated sizes is misleading, because it
    will double-count the contribution of the Operand of the func as
    well as the size of the whole TerminatorKind.

    • I did consider abandoning the MIR Visitor and instead hand-coding
      a traversal that distinguished embedded storage from indirect
      storage. But such code would be fragile; better to just require
      people to take care when interpreting the presented results.
  • This traverses the mir.promoted rvalues to capture stats for MIR
    stored there, even though the MIR visitor super_mir method does not
    do so. (I did not observe any promoted mir being newly traversed when
    compiling the rustc crate, however.)

  • It might be nice to try to unify this code with hir-stats. Then
    again, the reporting portion is the only common code (I think), and
    it is small compared to the visitors in hir-stats and mir-stats.

@rust-highfive
Copy link
Collaborator

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

Some notes:

* This code attempts to present the breakdown of each variant for
  every enum in the MIR. This is meant to guide decisions about how to
  revise representations e.g. when to box payloads for rare variants
  to shrink the size of the enum overall.

* I left out the "Total:" line that hir-stats presents, because this
  implementation uses the MIR Visitor infrastructure, and the memory
  usage of structures directly embedded in other structures (e.g. the
  `func: Operand` in a `TerminatorKind:Call`) is not distinguished
  from similar structures allocated in a `Vec` (e.g. the `args:
  Vec<Operand>` in a `TerminatorKind::Call`). This means that a naive
  summation of all the accumulated sizes is misleading, because it
  will double-count the contribution of the `Operand` of the `func` as
  well as the size of the whole `TerminatorKind`.

  * I did consider abandoning the MIR Visitor and instead hand-coding
    a traversal that distinguished embedded storage from indirect
    storage. But such code would be fragile; better to just require
    people to take care when interpreting the presented results.

* This traverses the `mir.promoted` rvalues to capture stats for MIR
  stored there, even though the MIR visitor super_mir method does not
  do so. (I did not observe any new mir being traversed when compiling
  the rustc crate, however.)

* It might be nice to try to unify this code with hir-stats.  Then
  again, the reporting portion is the only common code (I think), and
  it is small compared to the visitors in hir-stats and mir-stats.
@nrc
Copy link
Member

nrc commented Nov 30, 2016

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned nrc Nov 30, 2016
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 1, 2016

📌 Commit ff1ba6a has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 4, 2016

⌛ Testing commit ff1ba6a with merge ebeee0e...

bors added a commit that referenced this pull request Dec 4, 2016
Adds `-Z mir-stats`, which is similar to `-Z hir-stats`.

Adds `-Z mir-stats`, which is similar to `-Z hir-stats`.

Some notes:

* This code attempts to present the breakdown of each variant for
  every enum in the MIR. This is meant to guide decisions about how to
  revise representations e.g. when to box payloads for rare variants
  to shrink the size of the enum overall.

* I left out the "Total:" line that hir-stats presents, because this
  implementation uses the MIR Visitor infrastructure, and the memory
  usage of structures directly embedded in other structures (e.g. the
  `func: Operand` in a `TerminatorKind:Call`) is not distinguished
  from similar structures allocated in a `Vec` (e.g. the `args:
  Vec<Operand>` in a `TerminatorKind::Call`). This means that a naive
  summation of all the accumulated sizes is misleading, because it
  will double-count the contribution of the `Operand` of the `func` as
  well as the size of the whole `TerminatorKind`.

  * I did consider abandoning the MIR Visitor and instead hand-coding
    a traversal that distinguished embedded storage from indirect
    storage. But such code would be fragile; better to just require
    people to take care when interpreting the presented results.

* This traverses the `mir.promoted` rvalues to capture stats for MIR
  stored there, even though the MIR visitor super_mir method does not
  do so. (I did not observe any promoted mir being newly traversed when
  compiling the rustc crate, however.)

* It might be nice to try to unify this code with hir-stats.  Then
  again, the reporting portion is the only common code (I think), and
  it is small compared to the visitors in hir-stats and mir-stats.
@bors bors merged commit ff1ba6a into rust-lang:master Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants