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

Memory profiling #658

Merged
merged 5 commits into from
Oct 29, 2024
Merged

Conversation

themighty1
Copy link
Member

@themighty1 themighty1 commented Oct 28, 2024

This PR adds the functionality to profile memory usage of the native prover.
The gh workflow ran successfully here https://github.com/themighty1/tlsn/actions/runs/11553800582

This PR is a copy of #602

rebased on dev
reorganized files
fix gh workflow
Copy link

codecov bot commented Oct 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.48%. Comparing base (c6dc262) to head (f9fa5e3).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #658   +/-   ##
=======================================
  Coverage   54.48%   54.48%           
=======================================
  Files         193      193           
  Lines       20618    20618           
=======================================
  Hits        11234    11234           
  Misses       9384     9384           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sinui0
Copy link
Member

sinui0 commented Oct 28, 2024

Please be sure to squash the commits and add @valo as a co-author

Copy link
Member

@heeckhau heeckhau left a comment

Choose a reason for hiding this comment

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

Looking good!

@themighty1 themighty1 merged commit e6bc93c into tlsnotary:dev Oct 29, 2024
8 checks passed
@themighty1
Copy link
Member Author

I added a Co-authored-by: line but apparently github didn't pick it up. Sorry for that.

@themighty1 themighty1 deleted the memory-profiling-backup branch October 29, 2024 16:22
@heeckhau
Copy link
Member

@valo Thanks again for your great contribution 🙏

@valo
Copy link
Contributor

valo commented Oct 31, 2024

Oh no... no attribution in the git history 😢 A friendly recommendation to avoid this, as it will drive external contributors away. For example, I'll prob avoid making new contributions here from now on.

@themighty1
Copy link
Member Author

@valo , do you know what your Co-authored-by: line should look like (for future reference)
Im sorry that there was a chain of mishaps: you were not able to respond to my request here #602 (comment)
Then I waited for a week and decided to move forward. Then I tried to find your "Co-authored-by:" line in your previous commits, but I couldn't find it. Lesson learned.

We actually do encourage contributors and even go out of the way to atrribute their contributions.
Your future contributions are very welcome.

@valo
Copy link
Contributor

valo commented Oct 31, 2024

Im sorry that there was a chain of mishaps: you were not able to respond to my request here #602 (comment)

For some reason the emails for this issue were going to my spam folder :-( On the top of this, I was attending some events and not paying attention to GitHub that much. I should have paid more attention to this.

Co-authored-by: Valentin Mihov <valentin.mihov@gmail.com>

themighty1 added a commit that referenced this pull request Oct 31, 2024
* (squashing to simplify rebase)
rebased on dev
reorganized files
fix gh workflow

* modify workflow

* update dockerfile

Co-authored-by: Valentin Mihov <valentin.mihov@gmail.com>
themighty1 added a commit that referenced this pull request Oct 31, 2024
* (squashing to simplify rebase)
rebased on dev
reorganized files
fix gh workflow

* modify workflow

* update dockerfile

Co-authored-by: Valentin Mihov <valentin.mihov@gmail.com>
@themighty1
Copy link
Member Author

@valo , thanks
#660
is the best we could come up with. You are attributed now, albeit in a non-straightforward manner.

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