-
Notifications
You must be signed in to change notification settings - Fork 0
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
Any interest merging this code into remake? #1
Comments
I tried merging the profiling feature into the upstream make, but at the same time the support for parallel output was introduced and updating the code at the time to make it compatible with that was an effort I didn't had time for then. I would be very interested in merging both profiling and the graph features into both GNU Make and remake, especially if remake promises to remain backwards compatible with GNU make. How can we make this happen? |
As nike says, Just do it! I'm out of town for a week or so starting in a couple of hours I may have some time on a couple of 5-hour flights. If you could organize changes in a branch of your code as maybe one big squashed patch that would help me merge it in. Of course anyone can fork my code and submit a pull request against that. I've already cloned yours on my laptop. Thanks. |
So the branch should be forked of master from your remake repo? I will be busy with some other things until this weekend, but I think I can start the rebase on Saturday. Do you really want this to be squashed, I prefer the history to be preserved, at least to some degree. One reason being that I am not the only author of the changes. Since you already cloned, note that during my attempt to merge into GNU make the profiling output was trimmed and cut to the extreme, which I didn't agree with since it would reduce the usability of the new feature, and increase the work necessary after the execution of make/remake. I think you should look at the profile-fmtstring-rebase branch, I think the concept implemented there is closer to how I envisioned the feature to work to be most helpful. Any comments on the code are welcome. |
GIt is all about branches. In contrast to other version control systems, one can easily create then and destroy them just as easily. Anyone who wants to work in remake off of a branch rather than off of master won't get an argument from me. Others have found it easier to work off of master, so sometimes I am accomodating when working with others. Given that git branching is easy, I suggest making branch(es) in make-profiler repo which has squashed changes. This of course can be temporary. Or maybe 3 more branches. One branch is for just profiling, one for just graphing and one for profile+graph. If it so happens that there are inter-dependencies between profiling and graphing, the one-branch combined-feature branch might be less hassle. Those 1-3 branches don't change the master branch in make-profiler which contain the real deal with full history. (And if you really are afraid of master getting messed up you can create a branch now for that). I ask for the squash changes only to because it seems easier to merge this into remake. In remake each merge from make-profiler would be to a corresponding branch in remake. And then when all parties are happy, which might be a long ways away, everything would be merged back into master. After this proof of concept - that it can be done - then we can go back to the full history, and redo the 1 or 3 branches keeping portions you want expanded out, expanded. Does this make sense? If not, let's discuss. Talk is cheap and may be easier than working this out via hacking on it in the small. On Tue, May 19, 2015 at 3:39 AM, eddyp notifications@github.com wrote:
Ok. Will do.
Will do if when I have some. But as they say, help me help you bydistilling the changes if you can. I'm even cool with a doc in plain english or psuedo code as to what was or needs to be done. Thanks. —
|
I have now had the chance to build the profile-fmtstring-rebase branch and Overall I think the changes are pretty well-contained and I wouldn't Printing profile or graph info to stderr is weird. I'd prefer to have BTW, the dot graph that was produced when I ran make building itself was Given that the changes seem pretty well segregated and there are already What I think would be most beneficial is something like a design or I'd appreciate seeing how you go end-to-end to get that nice profile graph Thanks. On Tue, May 19, 2015 at 4:08 AM, Rocky Bernstein rocky.bernstein@gmail.com
|
Pe 19 mai 2015 2:03 p.m., "R. Bernstein" notifications@github.com a scris:
The output from the profiling related code could be broken/interrupted if I didn't had time back then to switch to the new APIs and I think all the Also I think the profiling output could be separated into its own file
I'll look into GCC and gprof to get some inspiration. I'm not fond of any
It is useful for very complex makefiles. I can provide an output from a The main use case is figuring out if specific targets could be parallelized
You can do it, I don't mind.
I understand, that is fine. Maybe the output doc/outline can be used to
Makes sense. Will keep it in mind when I start the work on Saturday. I'll try to rebase on top of remake/master, then create a bit of There isn't much in the graph feature, so the changes there aren't that
|
Just for reference, the ticket I opened to merge the profiling patch into make is here: http://savannah.gnu.org/bugs/?40639. The mailing list discussion around the proposal starts here: Roughly the comments from Paul were:
Of these comments, having my own experience when using the feature, I find the suggestion of using XML and having to implment scripts and external tools to process the data the least appealing and the one I find least practical/useful of the suggestions. |
On Wed, May 20, 2015 at 1:19 PM, eddyp notifications@github.com wrote:
My motivation there was just that this code complicated things way too much for me to understand how to change the code base and I had no way to test this. When I revised for GNU Make 4.1 I started again from the code base and since I now know what's needed, I think all of that code that I removed But my overall point was, this is not something I get hung up about. If you can handle it, great. If not well, having the feature outweighs not having it because it can't be supported everywhere. If the feature is there but not on WIndows and some WIndows person wants it, they can add it. I'm not criticising GNU Make or its decision. I'm just saying that thisproject is different. In fact, I want to promote this project as a mechanism where people can try out new and crazy ideas. And since it
On the other hand, sure absolutely I do care that work that is done by others is properly tagged as such. Git generally handles this properly though.
First of all, real experience counts for a lot over imagined experience using profileing in GNU Make. Quite frankly using XML feels to me like a disaster and going back 10 years. Nowadays if you wanted a common format, JSON would probably be what one uses. But using external tools to process data is I think the right thing. GNU Make is already too large and bloated. If I could have added the Think about how profiling is done in C. You run gcc -pg and that creates something that gprof uses. And as I've said before, it would be cool if whatever format gcc -pg produces, so does GNU make. So gprof could be used pretty much unaltered. Suppose gprof supports showing data as flame graphs? (If it doesn't already, it should, and might someday). Do you really want to write or your own implementation, or cut and past someone else's and insert that into GNU Make? You'd be fighting a losing technology battle if you did it this way. I am not wed to gprof per se. If there is another tool out there that functions like gprof (and probably there is) and that is preferable, fine; let's go with it. But again decoupling presentation of data from collection of data is definitely the way to go. But... as they say I vote with my feet. Because I feel strongly about this I am prepared to do the work to make it so. And this is yet another way that I feel this is different from GNU Make which is "default no" unless you can demonstrate "yes". —
|
Looking at this briefly an fight so I can't investigate fully right now, Again will investigate more fully later. On Thu, May 21, 2015 at 4:33 AM, Rocky Bernstein rocky.bernstein@gmail.com
|
I had a chance to look at in more detail which negates a lot of what I wrote above about the simplicity of filtering into gprof. mcount() records PC addresses which are assumed to be looked up later in the symbolic portion (e.g .text section) of a machine binary. That is why gprof needs two input files:
The basic information that is read in is done in gmon_out_read() of gprof/gmon_io.c. The two pieces of information that are relevant here are:
So one approach would be to have remake spit out this information, and then write another program that creates both a fake "binary" and encodes the above information into machine addresses. A bit ugly but doable. Another possibility is to look for something else like gprof. https://github.com/jrfonseca/gprof2dot/blob/master/README.markdown suggests other things that produce profiling output. |
callgrind output is a text file and I guess somewhat straightforward although I am not sure how to get information such call information in there.
|
I have looked through the What the feature does is to measure the execution of each of the targets (measurements not done at command level, but at target level) since that information can be obtained from make directly at runtime. I am rather skipping gprof entirely and generating the data almost fit for import into a tool such as LibreOffice Calc or MS Office Excel. See explanations here and the first workflow: I'll look into callgrind, too. |
I don't think importing into spreadsheet though is the way to go. Profiling is already its own art and beast. Find a front-end tool that displays profiling data and call-graph data. Profiling tools understand the difference between total time under this function (or target) versus time excluding other calls (or prerequisite targets before this one can be built). Both figures are important. Profiling tools sometimes have a threshold to below which you don't need to show data on. And a good profiling tool will also be able to show this in flame graph format. |
Looked a little more at callgrind and have revised the input example above. See https://gist.github.com/rocky/e946f17fc701ceb611af and run kcachegrind on the file with the Makefile in the same directory. |
After reading a little more carefully our messages and on Callgrind, I have a few comments:
Sorry if this looks like I don't want to change anything in my code, it's not that, it is just that I want to make sure we understand what problem we're trying to solve. |
No argument with that. Looking at both gprof and valgrind/callgrind, only two measurements are needed:
In addition to suggesting what information to gather, callgrind has been useful in suggesting what filename to use by default. Because of the topological sort nature of make, there will only be one call between any two targets. However a target can get tested more than once. The above information matches information you were collecting too, right? remake has the call information easily accessible since it's used in error reporting and debugging. Getting the time used to get a target up to date seems straightforward. Your code already does that, right? Now onto the external format to record this. I am leaning towards using the callgrind format. It is pretty easy to generate, understand, and it is in text format. The likelihood of that format changing in an incompatible way is pretty small since it used publicly. And even if it does change a little bit, will it change in a way that effects how we are using it given that we are not using a lot of it? I doubt that too. But even here, I doubt it would be hard to move along with it. I would be willing to bet that it changes far less than what both you and I have already had to deal with in GNU Make source code changes. So I'm prepared to deal with that. If it does change, folks will use the old version of kcachegrind and existing alternatives until things get ironed out. Which brings us to the question of... Are people are stuck with kcachegrind for visualization? No. If you looked at the gist and the other remarks, you'll see that another path to visualization is via gprof2dot. Despite the grprof in that project name and code, it works with callgrind format as well. And lastly there's the tool that valgrind uses, callgrind-annotate. In an ideal world, yes that API would be better documented. (If it mollifies you, think about the level of documentation for how to use your tool as of the changes before the last set of changes in the last couple of days.) However, it is easy to read callgrind-annotate, and glean how to parse the information. If you don't like Perl of callgrind-annotate, look at gprof2dot written in Python. That there are already 3 readers written in 3 programming languages, suggests that this isn't a hard format to grok. I am amenable to using some other format. The pointer to the readme in gprof2dot was to suggest other places to look. Or use the google for finding other possibilities. But let's hook into something else than reinventing the wheel now for the 10th time (judging from gprof2dot). I've started down the road of using callgrind. In a branch called profile on my local copy, I have created a single file, I am at the place where I would store that additional profiling data listed above. There are basically 3 choices
Outputing as you go feels to me the trickiest. If it turns out there is accumulated data, then you are hosed and need to do something along the second method. One thing of this ilk might be indicating whether the make run was successful or errored out, which of course you only know at the end but has to be toward the beginning of the file in The second option, using a new structure, is probably the most ideal since the profile information will probably touch only a fraction of all the targets that are used and even then, it is used only when profiling is in effect, which is rare. However, right now I'm thinking about extending the |
I have read your reply and I am mostly in line/agree with what you said. Regarding what the existing patch provides already, it is timing information and target information (name, pid of make process and recursion level), but we don't have yet the who-called-who part and the time spent evaluating and reevaluating existing targets. Also the time spent outside any targets (parsing the makefile and evaluating variables and expressions) is not directly available now, but could probably be obtained by subtracting from the total process execution time the time spent evaluating targets and the time spent in those targets (but that would probably be somewhat/entirely? incorrect since it would hide parallelism in evaluations) |
Thanks. This is good to hear.
callgrind_annotate can handle fixed-point numbers, but kcachegrind can't. But it is easy to turn this into an integer, perhaps a millisecond value.
The pid is captured and recorded once in the callgrind format as it should.
This can be derived from who calls who. And as I said, that information is already available in remake.
The callgrind format oddly doesn't have a way to report that time. However one way to do it would be to report that as the parent of the root target. If you give the simple total time elapsed the reporting programs do the subtraction I think. I don't think there is much more work to do on this or that it is difficult work, but things have been very busy for me and I just don't know when I'll have free time. |
In the profile branch of remake, there is a By the way, when I got around to adding timing data, the simplest thing is to use the POSIX time() function That returns seconds since epoc and is portable across lots of OS's. |
I wrote that i was using time() instead of gettimeofday() and I now see that the later has millisecond resolution whereas the former doesn't. So at some point I'll switch to that. As for Windows, again, someone with a vested interest in that can either revise to fallback to time() or something else. |
Pe 29 mai 2015 12:08 p.m., "R. Bernstein" notifications@github.com a
Looking at the code, is it me, is or are there some hard coded targets such
|
#ifdef STANDALONE I've also revised the gist to show current output. I am not sure why, |
Is this project dorman? Any interest in merging it into http://github.com/rocky/remake ?
The text was updated successfully, but these errors were encountered: