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

-dtimings should also report memory consumption #7514

Closed
vicuna opened this issue Apr 4, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@vicuna
Copy link

commented Apr 4, 2017

Original bug ID: 7514
Reporter: @gasche
Status: acknowledged (set by @damiendoligez on 2017-04-14T15:01:02Z)
Resolution: open
Priority: low
Severity: feature
Category: compiler driver

Bug description

Some of our users are investigating compilation issues where a blowup happen in memory usage of the compiler, rather than time. The -dtimings option is useful to help users extract information on the problematic pass for people trying to fix time blowup issues, having memory consumption information would help similarly debug memory issues.

After briefly looking at the Gc module documentation ( https://caml.inria.fr/pub/docs/manual-ocaml/libref/Gc.html ), I believe that the following information would be useful and could be always reported for each -dtimings output line:

  • the live_words value at the times the timer was started and stopped
  • the maximum top_heap_words value reached while the timer was active

One could consider having a more verbose output available, enabled by an extra flags (-dtimings-gc-stats?), which would collect (Gc.print_stat) calls on start/stop for each timer, but that can be left for later.

Additional information

(The reason why we want those measurements to be convenient for users to perform is that they have their problematic codebase at hand, and it may be difficult for them to extract an easy-to-build repro-case, and it may be difficult for compiler developers to build their codebase.)

@vicuna

This comment has been minimized.

Copy link
Author

commented Apr 4, 2017

Comment author: @gasche

In an email, Sébastien Hinderer wrote:

Well, if -dtimings starts to report memory-related infomration,
then I'd say that "timings" stops to be an appropriate name. So
since -dtimings is, AFAIK, not part of any official release
yet, perhaps it is still a good moment to rename it to
something more generic like -dstatistics? Or, if that's not
desired, perhaps memory-related info would diserve its
own -dmemory or whatever option?

I think that -dperfstats or something may indeed be a good idea. I would rather not have two separate -dtimings and -dmemory option, because I think it makes sense to have a single combined hierarchical printing of pass-per-pass information than two separate things being displayed. (But having more advanced options to ask for more specialized/verbose outputs would be reasonable.)

@vicuna

This comment has been minimized.

Copy link
Author

commented Apr 4, 2017

Comment author: @gasche

In fact I re-checked: -dtimings was released in 4.03 (it is #306, although it would be easier to find if the Changes entry explicitly mentioned the option name), so it's harder to change the name now.

@vicuna

This comment has been minimized.

Copy link
Author

commented Apr 5, 2017

Comment author: @chambart

I think it is ok to change the name of the option. It is a tool for compiler development, if anybody relied on it, they deserve to get their build broken. Especially given that the printing format changed, it would be better to break it in a clearly visible way.

As for the reported information, I think that the most important one is minor-words and major-words. Live words is too unreliable to if one does not trigger a complete collection (and we certainly don't want to do that by default).

@mshinwell

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2019

@gasche I think the "new" profiling support replacing -dtimings fixes this to a sufficient extent, right?

@gasche

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

Yes, -dprofile shows heap consumption so this can indeed be closed.

@gasche gasche closed this Mar 18, 2019

@vicuna vicuna added the feature-wish label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.