Profiling feature

[GeorgiosDelkos]

Description

This pull request adds the profile feature in the Sway compiler.

It communicates with an external forc-perf executable in order to signal the beginning and end of different compilation phases for profiling purposes.

Checklist

• [ ] I have linked to any relevant issues.
• [ ] I have commented my code, particularly in hard-to-understand areas.
• [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • [ ] If my change requires substantial documentation changes, I have requested support from the DevRel team
• [ ] I have added tests that prove my fix is effective or that my feature works.
• [ ] I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
• [ ] I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
• [x] I have requested a review from the relevant team or maintainers.

[CLAassistant]

All committers have signed the CLA.

[camden-smallwood]

Is the purpose of this profiler to profile sway code, or the rust code of the compiler itself? The name forc perf suggests the former, but it looks like this is attempting to do the latter.

Sorry if I'm missing the point here, I don't have much context on this project. Is there something that this tool does that we don't get from criterion + codspeed?

I could see the value here being that we can run it against multiple sway projects, but the maintenance of maintaining this tooling does not seem worth it. We already have a benchmark for the compile function which includes most if not all of the functions captured by time_expr. It would be good to add more benchmarks of the compile function over a diverse set of sway projects.

Please speak with Jibril and Nick about this, as we worked on this engagement in an external agreement with them.

You can find the forc-perf repository here: https://github.com/ourovoros-io/forc-perf

We opted to use an external profiling process so that the process-specific resource consumption is not affected by the resource consumption of the profiler itself. We added these changes in order to be able to delineate the individual phases.

[IGI-111]

@sdankel

Is the purpose of this profiler to profile sway code, or the rust code of the compiler itself?

Both, or rather it's meant to help us track the performance of the compiler both in compilation time and runtime performance.

Is there something that this tool does that we don't get from criterion + codspeed?

This has been in development for longer than we've added these, but the point is to get fine grained information about the compiler execution, including memory usage in various passes, statistics of instruction usage, etc.

Codspeed is nice but it doesn't really have the level of customization we're looking for for compiler benchmarks.

I could see the value here being that we can run it against multiple sway projects

This is precisely the goal, part of the point of this is to set up a representative sample of Sway to benchmark on a standardized machine so we can track compiler performance over time.

[codspeed-hq[bot]]

CodSpeed Performance Report

Merging #6565 will not alter performance

_{Comparing ourovoros-io:master (bc6a73e) with master (5b7d720)}

Summary

✅ 22 untouched benchmarks

[tritao]

Hi @GeorgiosDelkos, this currently has some conflicts, can you get this rebased on latest master?

[tritao]

@sdankel The perf tool is at https://github.com/ourovoros-io/forc-perf

I've been trying to test this with the following steps:

Build Sway with this PR: cargo run --release --bin forc --features profiler Run forc-perf: cargo run --release -- -t /dev/sway/test-case -f /dev/sway/target/release/forc

Some feedback / issues:

1. Usage and size metrics are mostly all reported as 0.

[
  [
    "/home/joao/dev/sway/test-case",
    {
      "cpu_usage": [
        0.0,
        0.0
      ],
      "memory_usage": [
        0.0,
        0.0
      ],
      "virtual_memory_usage": [
        0.0,
        0.0
      ],
      "disk_total_written_bytes": [
        0.0,
        0.0
      ],
      "disk_written_bytes": [
        0.0,
        0.0
      ],
      "disk_total_read_bytes": [
        0.0,
        0.0
      ],
      "disk_read_bytes": [
        0.0,
        0.0
      ],
      "bytecode_size": [
        0.0,
        0.0
      ],
      "data_section_size": [
        0.0,
        0.0
      ],
      "time": [
        -1.0,
        -7.692307692307693
      ]
    }
  ]
]

2. Verbose reporting even when not necessary.

On a working run, I got the following report which I find pretty verbose and just clutters the entire terminal without much useful info.

----------------------------------------------------------------------------------------------------
Metric: cpu_usage
Previous value: 0
Current value: 0
Change: 0
Percentage Change: 0%
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
Metric: memory_usage
Previous value: 0
Current value: 0
Change: 0
Percentage Change: 0%
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
Metric: virtual_memory_usage
Previous value: 0
Current value: 0
Change: 0
Percentage Change: 0%
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
Metric: disk_total_written_bytes
Previous value: 0
Current value: 0
Change: 0
Percentage Change: 0%
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
Metric: disk_written_bytes
Previous value: 0
Current value: 0
Change: 0
Percentage Change: 0%
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
Metric: disk_total_read_bytes
Previous value: 0
Current value: 0
Change: 0
Percentage Change: 0%
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
Metric: disk_read_bytes
Previous value: 0
Current value: 0
Change: 0
Percentage Change: 0%
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
Metric: bytecode_size
Previous value: 24
Current value: 24
Change: 0
Percentage Change: 0%
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
Metric: datasection_size
Previous value: 0
Current value: 0
Change: 0
Percentage Change: 0%
----------------------------------------------------------------------------------------------------
----------------------------------------------------------------------------------------------------
Metric: time
Previous value: 13
Current value: 12
Change: -1
Percentage Change: -7.692307692307693%
----------------------------------------------------------------------------------------------------

3. No error reporting seems to happen when passing an existing folder that does not contain a Sway project. For example, passing the src folder of a Sway project instead of the root folder containing forc.toml.

4. Not really an issue but the tool is kinda opaque, would be great if there was a --verbose flag that shows some output of what exactly the tool does under the hood, both for understanding and diagnostics.

[tritao]

@GeorgiosDelkos This still shows up some conflicts, can you please get this rebased on top of latest master?

[tritao]

@GeorgiosDelkos @camden-smallwood Still some issues with this, fmt check, a snapshot test failing and clippy.

[camden-smallwood]

@GeorgiosDelkos @camden-smallwood Still some issues with this, fmt check, a snapshot test failing and clippy.

These should be fixed now in the most recent commit.

[tritao]

@GeorgiosDelkos @camden-smallwood Still some issues with this, fmt check, a snapshot test failing and clippy.

These should be fixed now in the most recent commit.

Still seems to be failing on ---- should_pass/forc/help ----.

[camden-smallwood]

@GeorgiosDelkos @camden-smallwood Still some issues with this, fmt check, a snapshot test failing and clippy.

These should be fixed now in the most recent commit.

Still seems to be failing on ---- should_pass/forc/help ----.

I have attempted to remedy this again, but I must say that this particular test is extremely pedantic. All it's doing is checking the output of forc build -h, but the format that is in the snapshot is non-standard. If it fails again, I will have to request that someone from the Fuel team fix it instead, because the expected format does not match the output of forc build -h on my system.

[camden-smallwood]

After looking at this further and speaking with Jibril, I have determined that there are 2 issues with this test:

1. The output from the help command is formatted based on the size of the current terminal.
2. The options are generated using a derive macro on a Rust ABI struct. This means the layout of the struct is not stable or deterministic, which can lead to different field ordering, and in turn different output text.

I believe the help test needs to be refactored to be more deterministic, because it's quite difficult to reproduce outside of the CI.

[tritao]

After looking at this further and speaking with Jibril, I have determined that there are 2 issues with this test:

1. The output from the help command is formatted based on the size of the current terminal.
2. The options are generated using a derive macro on a Rust ABI struct. This means the layout of the struct is not stable or deterministic, which can lead to different field ordering, and in turn different output text.

I believe the help test needs to be refactored to be more deterministic, because it's quite difficult to reproduce outside of the CI.

Thanks for looking into this, seems like the most pragmatic solution right now is just so remove this test, opened a PR for that.

[tritao]

@camden-smallwood The test has now been removed. Can we get this rebased again to hopefully merge it this time?

EDIT: I was able to re-trigger the failing jobs, maybe that'll be enough?

[tritao]

Ok this needs a bit more work, closing this one to re-open a new PR since I have no access to the branch.