Skip to content

Conversation

@nickrobinson251
Copy link
Collaborator

No description provided.

@coveralls
Copy link

coveralls commented Dec 7, 2022

Pull Request Test Coverage Report for Build 3641572578

  • 38 of 38 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.361%

Totals Coverage Status
Change from base Build 3638355364: 0%
Covered Lines: 120
Relevant Lines: 122

💛 - Coveralls

@nickrobinson251

This comment was marked as outdated.

@nickrobinson251

This comment was marked as outdated.

Comment on lines 91 to 94
uri = HTTP.URI(req.target)
qp = HTTP.queryparams(uri)
with_pprof = parse(Bool, get(qp, "pprof", default_pprof()))
return _stop_cpu_profile(with_pprof)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to @quinnj's suggestion to stop the profile as soon as possible, and then do the rest of this work after it's stopped.

@nickrobinson251
Copy link
Collaborator Author

okay, i re-ran the profiling via the different endpoints (/profile versus /profile_start+/profile_stop) and see almost-identical profiles, so pretty sure my worry above was just due to poor execution of my experiments

Comment on lines +215 to +218
resp = HTTP.Response(200, "Allocation profiling started.")
Profile.Allocs.clear()
Profile.Allocs.start(; sample_rate)
return HTTP.Response(200, "Allocation profiling started.")
return resp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

genius

Copy link
Member

@NHDaly NHDaly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! :) Thanks @nickrobinson251

@nickrobinson251 nickrobinson251 merged commit 732cb0d into main Dec 7, 2022
@nickrobinson251 nickrobinson251 deleted the npr/cpu-start-stop branch December 7, 2022 18:22
@NHDaly
Copy link
Member

NHDaly commented Dec 8, 2022

Awesome! 😊

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3641552542

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 38 of 38 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on npr/cpu-start-stop at 98.361%

Totals Coverage Status
Change from base Build 3638355364: 98.4%
Covered Lines: 120
Relevant Lines: 122

💛 - Coveralls

@coveralls
Copy link

coveralls commented Aug 6, 2025

Pull Request Test Coverage Report for Build 3638763327

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 35 of 39 (89.74%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on npr/cpu-start-stop at 95.161%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/PerformanceProfilingHttpEndpoints.jl 35 39 89.74%
Totals Coverage Status
Change from base Build 3638355364: 95.2%
Covered Lines: 118
Relevant Lines: 124

💛 - Coveralls

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.

4 participants