Summary: | Add a simple progress-reporting facility | ||
---|---|---|---|
Product: | [Developer tools] valgrind | Reporter: | Julian Seward <jseward> |
Component: | general | Assignee: | Julian Seward <jseward> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | ivosh |
Priority: | NOR | ||
Version: | 3.14 SVN | ||
Target Milestone: | --- | ||
Platform: | Other | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Sentry Crash Report: | |||
Attachments: |
Implementation.
Addresses all review comments in comments 2 and 3. Patch with Solaris implementation added |
Description
Julian Seward
2017-09-12 20:11:49 UTC
Created attachment 107830 [details]
Implementation.
This works on Linux. It should work on Solaris and OSX too, with the
limitation that it always shows zero user time. This is because UInt VG_(get_user_milliseconds) isn't implemented on those targets and so
just returns zero.
Thank you for the patch, Julian. I will implement VG_(get_user_milliseconds)() for Solaris, that's not a problem. However I wonder why allowed values for --progress-interval are only 1..3600? Why not 0, the default? Please also do something with comments around next_report_due_at, they look really scary. This is not the first static variable we have in Valgrind... What happens if a Valgrind'ed process with --progress-interval > 0 is suspended for several minutes? Is the logic in maybe_progress_report() able to cope with this? And the last nit - please use %u for "#thr" formatting instead of "%d". nThreads is UInt. And also please add the line explanation somewhere. User manual, verbose output... Created attachment 109872 [details]
Addresses all review comments in comments 2 and 3.
Addresses all review comments in comments 2 and 3. Ivo, if you are
happy with it, could you please do the Solaris version of
VG_(get_user_milliseconds) ?
Note, this patch stores millisecond values in a UInt (unsigned 32 bit int)
and so will overflow after 49.71 days. I don't consider this to be a big
problem.
(In reply to Julian Seward from comment #4) > Addresses all review comments in comments 2 and 3. Ivo, if you are > happy with it, could you please do the Solaris version of > VG_(get_user_milliseconds)? Will do that, Julian. Created attachment 109992 [details]
Patch with Solaris implementation added
Hi Julian, patch with Solaris implementation is attached. The patch has been refreshed to apply cleanly to the latest git HEAD and a NEWS entry added. It is ready to be pushed - let me know if I should do it. Ivo, thanks for the fix. Yes, if you are happy with it, please do push it. Pushed as commit bd077baa71a40b60dcf0286b9fb89d803323fd93. https://sourceware.org/git/?p=valgrind.git;a=commit;h=bd077baa71a40b60dcf0286b9fb89d803323fd93 (In reply to Ivo Raisr from comment #9) > Pushed as commit bd077baa71a40b60dcf0286b9fb89d803323fd93. Thanks! |