Patchwork [v4] ccache: expose control interface via 'make ccache-options'

login
register
mail settings
Submitter Tzu-Jung Lee
Date Oct. 10, 2013, 5:50 p.m.
Message ID <1381427457-25627-1-git-send-email-tjlee@ambarella.com>
Download mbox | patch
Permalink /patch/282418/
State Accepted
Headers show

Comments

Tzu-Jung Lee - Oct. 10, 2013, 5:50 p.m.
usage:
	# set cache limit size
	make CCACHE_OPTIONS="--max-size=5G" ccache-options

	# zero statistics counters
	make CCACHE_OPTIONS="--zero-stats" ccache-options

Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com>
---
Include the useful warning message suggested by Thomas.

We might need a separate patch to obsolete the ccache-stats later.

 docs/manual/ccache-support.txt | 11 +++++++++++
 package/ccache/ccache.mk       | 11 +++++++++++
 2 files changed, 22 insertions(+)
Thomas De Schampheleire - Oct. 10, 2013, 7:45 p.m.
Hi Tzu-Jung,

Tzu-Jung Lee <roylee17@gmail.com> wrote:
>usage:
>	# set cache limit size
>	make CCACHE_OPTIONS="--max-size=5G" ccache-options
>
>	# zero statistics counters
>	make CCACHE_OPTIONS="--zero-stats" ccache-options
>
>Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com>
>---
>Include the useful warning message suggested by Thomas.
>
>We might need a separate patch to obsolete the ccache-stats later.
>
> docs/manual/ccache-support.txt | 11 +++++++++++
> package/ccache/ccache.mk       | 11 +++++++++++
> 2 files changed, 22 insertions(+)
>
>diff --git a/docs/manual/ccache-support.txt b/docs/manual/ccache-support.txt
>index 4969180..fe06a01 100644
>--- a/docs/manual/ccache-support.txt
>+++ b/docs/manual/ccache-support.txt
>@@ -23,3 +23,14 @@ remove this directory.
> 
> You can get statistics on the cache (its size, number of hits,
> misses, etc.) by running +make ccache-stats+.
>+
>+The make target +ccache-options+ and the +CCACHE_OPTIONS+ variable
>+provide more generic access to the ccache. For example
>+
>+-----------------
>+# set cache limit size
>+make CCACHE_OPTIONS="--max-size=5G" ccache-options
>+
>+# zero statistics counters
>+make CCACHE_OPTIONS="--zero-stats" ccache-options
>+-----------------
>diff --git a/package/ccache/ccache.mk b/package/ccache/ccache.mk
>index c5e9385..7b6155d 100644
>--- a/package/ccache/ccache.mk
>+++ b/package/ccache/ccache.mk
>@@ -45,3 +45,14 @@ ifeq ($(BR2_CCACHE),y)
> ccache-stats: host-ccache
> 	$(Q)$(CCACHE) -s
> endif
>+
>+ifeq ($(BR2_CCACHE),y)
>+ccache-options: host-ccache
>+ifeq ($(CCACHE_OPTIONS),)
>+	$(Q)echo "Usage: make ccache-options CCACHE_OPTIONS=\"opts\""
>+	$(Q)echo "where 'opts' corresponds to one or more valid ccache options" \
>+	"(see ccache help text below)"
>+	$(Q)echo
>+endif
>+	$(Q)$(CCACHE) $(CCACHE_OPTIONS)
>+endif

Did you see my other comment about combining the ifeq statements of ccache-stats and ccache-options?

Best regards,
Thomas
Tzu-Jung Lee - Oct. 10, 2013, 7:51 p.m.
Hi Thomas,

On Thu, Oct 10, 2013 at 12:45 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> Hi Tzu-Jung,
>
> Tzu-Jung Lee <roylee17@gmail.com> wrote:
>>usage:
>>       # set cache limit size
>>       make CCACHE_OPTIONS="--max-size=5G" ccache-options
>>
>>       # zero statistics counters
>>       make CCACHE_OPTIONS="--zero-stats" ccache-options
>>
>>Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com>
>>---
>>Include the useful warning message suggested by Thomas.
>>
>>We might need a separate patch to obsolete the ccache-stats later.
>>
>> docs/manual/ccache-support.txt | 11 +++++++++++
>> package/ccache/ccache.mk       | 11 +++++++++++
>> 2 files changed, 22 insertions(+)
>>
>>diff --git a/docs/manual/ccache-support.txt b/docs/manual/ccache-support.txt
>>index 4969180..fe06a01 100644
>>--- a/docs/manual/ccache-support.txt
>>+++ b/docs/manual/ccache-support.txt
>>@@ -23,3 +23,14 @@ remove this directory.
>>
>> You can get statistics on the cache (its size, number of hits,
>> misses, etc.) by running +make ccache-stats+.
>>+
>>+The make target +ccache-options+ and the +CCACHE_OPTIONS+ variable
>>+provide more generic access to the ccache. For example
>>+
>>+-----------------
>>+# set cache limit size
>>+make CCACHE_OPTIONS="--max-size=5G" ccache-options
>>+
>>+# zero statistics counters
>>+make CCACHE_OPTIONS="--zero-stats" ccache-options
>>+-----------------
>>diff --git a/package/ccache/ccache.mk b/package/ccache/ccache.mk
>>index c5e9385..7b6155d 100644
>>--- a/package/ccache/ccache.mk
>>+++ b/package/ccache/ccache.mk
>>@@ -45,3 +45,14 @@ ifeq ($(BR2_CCACHE),y)
>> ccache-stats: host-ccache
>>       $(Q)$(CCACHE) -s
>> endif
>>+
>>+ifeq ($(BR2_CCACHE),y)
>>+ccache-options: host-ccache
>>+ifeq ($(CCACHE_OPTIONS),)
>>+      $(Q)echo "Usage: make ccache-options CCACHE_OPTIONS=\"opts\""
>>+      $(Q)echo "where 'opts' corresponds to one or more valid ccache options" \
>>+      "(see ccache help text below)"
>>+      $(Q)echo
>>+endif
>>+      $(Q)$(CCACHE) $(CCACHE_OPTIONS)
>>+endif
>
> Did you see my other comment about combining the ifeq statements of ccache-stats and ccache-options?

Yes, but I think it probably would be better as a separate patch, which:

  1. either remove the ccache-stats code & and manual section
  2. or issue warning about the obsolete usage.

So this patch alone don't affect those are currently using
ccache-stats before they migrate to ccache-options.

thanks.

Roy
Thomas Petazzoni - Oct. 10, 2013, 8:57 p.m.
Dear Tzu-Jung Lee,

On Thu, 10 Oct 2013 12:51:27 -0700, Tzu-Jung Lee wrote:
> > Did you see my other comment about combining the ifeq statements of
> > ccache-stats and ccache-options?
> 
> Yes, but I think it probably would be better as a separate patch,
> which:
> 
>   1. either remove the ccache-stats code & and manual section
>   2. or issue warning about the obsolete usage.
> 
> So this patch alone don't affect those are currently using
> ccache-stats before they migrate to ccache-options.

I believe there might be a misunderstanding here. You seem to think
Thomas suggested to remove ccache-stats because ccache-options now
allows to do the same.

But in fact, Thomas only suggested to change:

"""
ifeq ($(BR2_CCACHE),y)
ccache-stats:
	...
endif

ifeq ($(BR2_CCACHE),y)
ccache-options:
	...
endif
"""

by

"""
ifeq ($(BR2_CCACHE),y)
ccache-stats:
	...

ccache-options:
	...
endif
"""

i.e, ccache-stats continues to exist, the only difference with your
implementation is that it sits under the same ifeq ($(BR2_CCACHE),y)
condition. It's just a minor nit.

Best regards,

Thomas
Tzu-Jung Lee - Oct. 10, 2013, 9:01 p.m.
On Thu, Oct 10, 2013 at 1:57 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Tzu-Jung Lee,
>
> On Thu, 10 Oct 2013 12:51:27 -0700, Tzu-Jung Lee wrote:
>> > Did you see my other comment about combining the ifeq statements of
>> > ccache-stats and ccache-options?
>>
>> Yes, but I think it probably would be better as a separate patch,
>> which:
>>
>>   1. either remove the ccache-stats code & and manual section
>>   2. or issue warning about the obsolete usage.
>>
>> So this patch alone don't affect those are currently using
>> ccache-stats before they migrate to ccache-options.
>
> I believe there might be a misunderstanding here. You seem to think
> Thomas suggested to remove ccache-stats because ccache-options now
> allows to do the same.
>
> But in fact, Thomas only suggested to change:
>
> """
> ifeq ($(BR2_CCACHE),y)
> ccache-stats:
>         ...
> endif
>
> ifeq ($(BR2_CCACHE),y)
> ccache-options:
>         ...
> endif
> """
>
> by
>
> """
> ifeq ($(BR2_CCACHE),y)
> ccache-stats:
>         ...
>
> ccache-options:
>         ...
> endif
> """
>
> i.e, ccache-stats continues to exist, the only difference with your
> implementation is that it sits under the same ifeq ($(BR2_CCACHE),y)
> condition. It's just a minor nit.

Oops, my bad.
Could you help do the inline editing when you merge it?

thanks.

Roy

>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
Thomas Petazzoni - Oct. 10, 2013, 9:05 p.m.
Dear Tzu-Jung Lee,

On Thu, 10 Oct 2013 14:01:40 -0700, Tzu-Jung Lee wrote:

> > i.e, ccache-stats continues to exist, the only difference with your
> > implementation is that it sits under the same ifeq ($(BR2_CCACHE),y)
> > condition. It's just a minor nit.
> 
> Oops, my bad.
> Could you help do the inline editing when you merge it?

I'm not the maintainer, so I can't merge patches. But most likely,
Peter Korsgaard can do it when merging your patch.

Thanks a lot for all your quick updates to this patch!

Best regards,

Thomas
Thomas De Schampheleire - Oct. 11, 2013, 4:52 a.m.
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
>Dear Tzu-Jung Lee,
>
>On Thu, 10 Oct 2013 14:01:40 -0700, Tzu-Jung Lee wrote:
>
>> > i.e, ccache-stats continues to exist, the only difference with your
>> > implementation is that it sits under the same ifeq ($(BR2_CCACHE),y)
>> > condition. It's just a minor nit.
>> 
>> Oops, my bad.
>> Could you help do the inline editing when you merge it?
>
>I'm not the maintainer, so I can't merge patches. But most likely,
>Peter Korsgaard can do it when merging your patch.
>
>Thanks a lot for all your quick updates to this patch!

Yes, I certainly agree with that!

Acked-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Peter Korsgaard - Oct. 27, 2013, 9:34 a.m.
>>>>> "Tzu-Jung" == Tzu-Jung Lee <roylee17@gmail.com> writes:

 > usage:
 > 	# set cache limit size
 > 	make CCACHE_OPTIONS="--max-size=5G" ccache-options

 > 	# zero statistics counters
 > 	make CCACHE_OPTIONS="--zero-stats" ccache-options

 > Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com>

Committed with the redundant ifeq removed as pointed out by Thomas,
thanks.

Patch

diff --git a/docs/manual/ccache-support.txt b/docs/manual/ccache-support.txt
index 4969180..fe06a01 100644
--- a/docs/manual/ccache-support.txt
+++ b/docs/manual/ccache-support.txt
@@ -23,3 +23,14 @@  remove this directory.
 
 You can get statistics on the cache (its size, number of hits,
 misses, etc.) by running +make ccache-stats+.
+
+The make target +ccache-options+ and the +CCACHE_OPTIONS+ variable
+provide more generic access to the ccache. For example
+
+-----------------
+# set cache limit size
+make CCACHE_OPTIONS="--max-size=5G" ccache-options
+
+# zero statistics counters
+make CCACHE_OPTIONS="--zero-stats" ccache-options
+-----------------
diff --git a/package/ccache/ccache.mk b/package/ccache/ccache.mk
index c5e9385..7b6155d 100644
--- a/package/ccache/ccache.mk
+++ b/package/ccache/ccache.mk
@@ -45,3 +45,14 @@  ifeq ($(BR2_CCACHE),y)
 ccache-stats: host-ccache
 	$(Q)$(CCACHE) -s
 endif
+
+ifeq ($(BR2_CCACHE),y)
+ccache-options: host-ccache
+ifeq ($(CCACHE_OPTIONS),)
+	$(Q)echo "Usage: make ccache-options CCACHE_OPTIONS=\"opts\""
+	$(Q)echo "where 'opts' corresponds to one or more valid ccache options" \
+	"(see ccache help text below)"
+	$(Q)echo
+endif
+	$(Q)$(CCACHE) $(CCACHE_OPTIONS)
+endif