Bug 123701 – Added CCache stats to the build-webkit perl script

• Component: Tools / Tests
• Status: RESOLVED WONTFIX

[Xabier Rodríguez Calvar]

Created attachment 215855 [details]
Patch

Added CCache stats to the build-webkit perl script.

[Xabier Rodríguez Calvar]

Added CCache stats to the build-webkit perl script

[Alexey Proskuryakov]

Perhaps it's just me being ignorant, but it would be helpful to see an explanation of what CCache is, and what the rationale is for this patch. It should probably be somewhere in the code too, not only in this bug.

[Xabier Rodríguez Calvar]

(In reply to comment #2)
> Perhaps it's just me being ignorant, but it would be helpful to see an explanation of what CCache is, and what the rationale is for this patch. It should probably be somewhere in the code too, not only in this bug.

ccache is compiler cache. When it is enable and you compile, it supplants the real compiler and perform some checks before actually compiling. If it happens to find (by hashing before and after preprocessing) the cached object is provided and compilation can be much faster. You can find more information at http://ccache.samba.org/.

It's quite recommended when building WebKit because it makes WebKit some compilations much faster with a very small overhead when the cache is missed.

I thought it could be interesting to see some stats after the compilation and that's why I cooked this patch. If you think it is interesting to land it, I would be more than happy of writing that comment in the code that you miss.

The patch is harmless when no ccache is found and nothing is printed in that case.

[Xabier Rodríguez Calvar]

Created attachment 216041 [details]
Patch

Added comment as requested.

[Xabier Rodríguez Calvar]

(In reply to comment #2)
> Perhaps it's just me being ignorant, but it would be helpful to see an explanation of what CCache is, and what the rationale is for this patch. It should probably be somewhere in the code too, not only in this bug.

Would it by any chance be possible that this gets reviewed?

[Alexey Proskuryakov]

Definitely not by me (can't review a patch for something I never heard of and that doesn't even exist on the platform I use!)

[Brent Fulgham]

This needs to be reviewed by someone on a GCC-based platform.  So can you find someone from the GTK+ or  maybe EFL project to review?

[Brent Fulgham]

Comment on attachment 216041 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=216041&action=review

This patch seems pretty harmless; it looks like it just reports some statistics when cache is in use.

> Tools/Scripts/build-webkit:78
> +    return %ccache unless system("ccache -s 2> /dev/null > /dev/null") == 0;

Is cache always available via the standard path? Do we need to use a $ccache variable to store the location on the user's system?

[Brent Fulgham]

Comment on attachment 216041 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=216041&action=review

This patch seems pretty harmless; it looks like it just reports some statistics when cache is in use.

> Tools/Scripts/build-webkit:78
> +    return %ccache unless system("ccache -s 2> /dev/null > /dev/null") == 0;

Is cache always available via the standard path? Do we need to use a $ccache variable to store the location on the user's system?

[Brent Fulgham]

I think this patch looks fine, but I would like mrobinson or someone else on the GTK (or even EFL) side to confirm it won't be an issue.

[Alexey Proskuryakov]

Another thing that needs to be confirmed is whether the code is actually useful enough to keep in trunk. Is it going to be used often, and to significant benefit?

[Xabier Rodríguez Calvar]

(In reply to comment #10)
> I think this patch looks fine, but I would like mrobinson or someone else on the GTK (or even EFL) side to confirm it won't be an issue.

I already CCed Martin.

[Xabier Rodríguez Calvar]

(In reply to comment #9)
> (From update of attachment 216041 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=216041&action=review
> 
> This patch seems pretty harmless; it looks like it just reports some statistics when cache is in use.

That's what I tried to do, to make it harmless and invisible for platforms not using ccache.

> > Tools/Scripts/build-webkit:78
> > +    return %ccache unless system("ccache -s 2> /dev/null > /dev/null") == 0;
> 
> Is cache always available via the standard path? Do we need to use a $ccache variable to store the location on the user's system?

It is in all distros that I know :)

[Xabier Rodríguez Calvar]

(In reply to comment #11)
> Another thing that needs to be confirmed is whether the code is actually useful enough to keep in trunk. Is it going to be used often, and to significant benefit?

Almost all WebKit developers that I know install ccache because compiling WebKit in a personal computer takes some time. Usually in Linux systems people combine ccache and (icecc or distcc) to cache and distribute compilation among different machines and make it faster.

I would say that it is as interesting as the time. It is not essential, but it is nice to see it and it gives you a very rough idea of how much code has changed since your last compilation. A result for a build I just finished:

====================================================================
 WebKit is now built (43m:58s). 
 CCache stats: 189 direct hits (5%), 6 preprocessed hits (0%),
	3893 misses (95%), 4088 total. 
 To run GtkLauncher with this newly-built code, use the
 "Tools/Scripts/run-launcher" script.
====================================================================

[Gustavo Noronha (kov)]

Comment on attachment 216041 [details]
Patch

I think it's a fun stat to have for curiosity's sake, much like the buildTime one. That said, it will most likely be wrong for me most of the time, though, because of the way it's implemented (I often build other stuff while going through a full build of webkit, say), and I'm not sure how much it adds in terms of usefulness. One can easily get that stat by doing what this patch does manually (though, arguably, the same can be said for the build time one). 

In summary, I don't mind having it, but I don't really think it's worth the additional complexity.

[Xabier Rodríguez Calvar]

(In reply to comment #15)
> (From update of attachment 216041 [details])
> I think it's a fun stat to have for curiosity's sake, much like the buildTime one. That said, it will most likely be wrong for me most of the time, though, because of the way it's implemented (I often build other stuff while going through a full build of webkit, say), and I'm not sure how much it adds in terms of usefulness. One can easily get that stat by doing what this patch does manually (though, arguably, the same can be said for the build time one). 

Yes, the problem of the accuracy is known but I couldn't think of an accurate solution without increasing the complexity too much. Ideas are welcome. :)

About the time what you say it is true and it is also true that it is also wrong some times when you for example suspend your laptop, and go home and then resume the build.

Anyway, as you say, my motivation was fun, mainly :)
 
> In summary, I don't mind having it, but I don't really think it's worth the additional complexity.

That's was my concern too.

[Michael Catanzaro]

Comment on attachment 216041 [details]
Patch

I'm not strongly opposed to this patch, but I'd rather not add it. As Gustavo notes, these stats are systemwide and will be inaccurate if you compile anything else while building WebKit. Moreover, it's not really interesting to me to see detailed stats, not nearly as interesting as showing the time. It's pretty obvious when you're getting cache hits, because the build will go 1000x faster than it usually does....

[Xabier Rodríguez Calvar]

(In reply to comment #17)
> Comment on attachment 216041 [details]
> Patch
> 
> I'm not strongly opposed to this patch, but I'd rather not add it. As
> Gustavo notes, these stats are systemwide and will be inaccurate if you
> compile anything else while building WebKit. Moreover, it's not really
> interesting to me to see detailed stats, not nearly as interesting as
> showing the time. It's pretty obvious when you're getting cache hits,
> because the build will go 1000x faster than it usually does....

I had already forgotten about this bug. Closing