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

Show the memory usage and time to build a crate #878

Open
jyn514 opened this issue Jul 4, 2020 · 5 comments
Open

Show the memory usage and time to build a crate #878

jyn514 opened this issue Jul 4, 2020 · 5 comments
Labels
A-builds Area: Building the documentation for a crate E-medium Effort: This requires a fair amount of work P-low Low priority issues

Comments

@jyn514
Copy link
Member

jyn514 commented Jul 4, 2020

For crates which time out due to resource limits, there is very little indication of why: the logs show only 'process exited with signal SIGKILL'. It would be ideal to say why the process was killed and how much time and memory were used.

I wonder if docs.rs can use this same trick with /usr/bin/time -v?

$ cargo clean
$ /usr/bin/time -v cargo doc --no-deps -p termwiz
...
        Command being timed: "cargo doc --no-deps -p termwiz"
        User time (seconds): 90.74
        System time (seconds): 6.77
        Percent of CPU this job got: 456%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:21.37
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 3810736
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 0
        Minor (reclaiming a frame) page faults: 2504493
        Voluntary context switches: 4301
        Involuntary context switches: 50046
        Swaps: 0
        File system inputs: 0
        File system outputs: 705048
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0

Originally posted by @wez in #842 (comment)

@jyn514 jyn514 added A-builds Area: Building the documentation for a crate P-low Low priority issues S-needs-design Status: There's a problem here, but no obvious solution; or the solution raises other questions labels Jul 4, 2020
@jyn514
Copy link
Member Author

jyn514 commented Jul 4, 2020

This needs changes to rustwide, see #842 (comment)

@pietroalbini
Copy link
Member

If the goal is just to alert the user whether the build failed due to a OOM or a timeout, we don't need to wrap the build with time: Rustwide already returns those reasons when a build fails.

We could change the build_status column in the database from a boolean to an enum/string:

  • succeeded
  • failed
  • failed-oom
  • failed-timeout

...and then provide a UI in the build log page displaying a warning if the status is failed-oom or failed-timeout`.

@jyn514 jyn514 added E-easy Effort: Should be easy to implement and would make a good first PR and removed S-needs-design Status: There's a problem here, but no obvious solution; or the solution raises other questions labels Jul 6, 2020
@Nemo157
Copy link
Member

Nemo157 commented Aug 27, 2020

Even for successful builds, it'd be nice to record the duration and max-memory usage and show those on the build page.

@wambu-i
Copy link

wambu-i commented Oct 8, 2020

Can I work on this?

@jyn514
Copy link
Member Author

jyn514 commented Oct 8, 2020

@wambu-i go for it! I think the steps will looks something like this:

  1. command.run() gives you a CommandError when the build fails:
    let successful = logging::capture(&storage, || {
    self.prepare_command(build, target, metadata, limits, rustdoc_flags)
    .and_then(|command| command.run().map_err(failure::Error::from))
    .is_ok()
    });
  2. Add space for it in BuildResult - I'd change successful: bool to build_error: Option<CommandError> and change all the current successful uses to build_error.is_none().
  3. Add a new migration to the database which stores the reason it failed:
    migration!(
  4. Update the builds page to have the new info:
    <div class="pure-u-1 pure-u-sm-1-24 build">
    {%- if build.build_status -%}
    {{ "check" | fas }}
    {%- else -%}
    {{ "times" | fas }}
    {%- endif -%}
    </div>
  5. That template is rendered at
    fn map_to_release(conn: &mut Client, crate_id: i32, version: semver::Version) -> Release {

Recording this info for successful builds will need rustwide changes, so let's leave those for later.

#1072 made similar changes and might be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-builds Area: Building the documentation for a crate E-medium Effort: This requires a fair amount of work P-low Low priority issues
Projects
None yet
Development

No branches or pull requests

4 participants