Skip to contentSkip to conversation

scripts/coccicheck: add paramap and indexing options

PATCH Superseded Luis R. Rodriguez 4 participants 45 messages Jun 10, 2016 – Jun 15, 2016 Coccinelle/Semantic Patches (SmPL)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

cover scripts/coccicheck: add paramap and indexing options

coccicheck hasn't been updated for a while. The backports project has been using some features for a while now that we should be able to also take advantage of with coccicheck, the most important one is paramap support.

glimpseindex stuff wasn't even building but today I decided to go tackle and fix that, the public open source release is is now working and you can optionally use that. Note that using git performs just as well, glimpseindex just shaves off a bit of time, however if since we can support it now we do it.

Luis R. Rodriguez (4):
coccicheck: move spatch binary check up
coccicheck: enable paramap support
scripts: add glimpse.sh for indexing the kernel
coccicheck: add indexing enhancement options

 scripts/coccicheck | 62 +++++++++++++++++++++++++++++++++++++++++++++++-------
 scripts/glimpse.sh | 12 +++++++++++
 2 files changed, 66 insertions(+), 8 deletions(-)
 create mode 100755 scripts/glimpse.sh
Signature · 1 line
2.8.2
#1 lore raw

PATCH 1/4 coccicheck: move spatch binary check up

This has no functional changes. This is being done to enable us to later use spatch binary for some flag checking for certain features early on.

---
 scripts/coccicheck | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
scripts/coccicheck +5 -5
index dd85a455b2ba..aa5e78fba270 100755
@@ -7,6 +7,11 @@
 
 SPATCH="`which ${SPATCH:=spatch}`"
 
+if [ ! -x "$SPATCH" ]; then
+    echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/'
+    exit 1
+fi
+
 trap kill_running SIGTERM SIGINT
 declare -a SPATCH_PID
 
@@ -51,11 +56,6 @@ if [ "$KBUILD_EXTMOD" != "" ] ; then
     OPTIONS="--patch $srctree $OPTIONS"
 fi
 
-if [ ! -x "$SPATCH" ]; then
-    echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/'
-    exit 1
-fi
-
 if [ "$MODE" = "" ] ; then
     if [ "$ONLINE" = "0" ] ; then
 	echo 'You have not explicitly specified the mode to use. Using default "report" mode.'
Signature · 1 line
2.8.2
#2 lore raw

PATCH 2/4 coccicheck: enable paramap support

In reply to Luis R. Rodriguez

Coccinelle has had paramap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

If --jobs is not supported we fallback to the old mechanism.

As a small example, prior to this change, on an 8-core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report
...
real    29m14.912s
user    103m1.796s
sys     0m4.464s

After:

real    16m22.435s
user    128m30.060s
sys     0m2.712s
---
 scripts/coccicheck | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)
scripts/coccicheck +25 -3
index aa5e78fba270..eeb5fdc142ca 100755
@@ -12,8 +12,8 @@ if [ ! -x "$SPATCH" ]; then
     exit 1
 fi
 
-trap kill_running SIGTERM SIGINT
-declare -a SPATCH_PID
+USE_JOBS="no"
+$SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes"
 
 # The verbosity may be set by the environmental parameter V=
 # as for example with 'make V=1 coccicheck'
@@ -82,7 +82,21 @@ if [ "$ONLINE" = "0" ] ; then
     echo ''
 fi
 
-run_cmd() {
+if [ "$USE_JOBS" = "no" ]; then
+	trap kill_running SIGTERM SIGINT
+	declare -a SPATCH_PID
+else
+	OPTIONS="$OPTIONS --jobs $NPROC --chunksize 1"
+fi
+
+run_cmd_paramap() {
+	if [ $VERBOSE -ne 0 ] ; then
+		echo "Running ($NPROC in parallel): $@"
+	fi
+	$@
+}
+
+run_cmd_old() {
 	local i
 	if [ $VERBOSE -ne 0 ] ; then
 		echo "Running ($NPROC in parallel): $@"
@@ -97,6 +111,14 @@ run_cmd() {
 	wait
 }
 
+run_cmd() {
+	if [ "$USE_JOBS" = "yes" ]; then
+		run_cmd_paramap $@
+	else
+		run_cmd_old $@
+	fi
+}
+
 kill_running() {
 	for i in $(seq 0 $(( NPROC - 1 )) ); do
 		if [ $VERBOSE -eq 2 ] ; then
Signature · 1 line
2.8.2
#3 lore raw

Re: [Cocci] [PATCH 2/4] coccicheck: enable paramap support

Quoted 2 lines

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Is this functionality influenced by the parameter "chunksize"?

Regards,
Markus

#4 lore raw

Re: [Cocci] [PATCH 2/4] coccicheck: enable paramap support

Quoted 4 lines · SF Markus Elfring
Quoted 2 lines

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Is this functionality influenced by the parameter "chunksize"?

Yes, without chunksize the distribution of work to processes is static.

julia

#5 lore raw

PATCH 3/4 scripts: add glimpse.sh for indexing the kernel

In reply to Luis R. Rodriguez

Glimpse is a tool you can use to index the kernel. The tool was recently open sourced under the ISC license and can be obtained at:

https://github.com/mcgrof/glimpse.git

---
 scripts/glimpse.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100755 scripts/glimpse.sh
scripts/glimpse.sh new file +12 -0
new file mode 100755
index 000000000000..5fd24e49f31b
@@ -0,0 +1,12 @@
+#!/bin/bash
+
+DIR=$(dirname $(readlink -f $0))
+DIR="${DIR}/../"
+
+GLIMPSEINDEX="`which ${GLIMPSEINDEX:=glimpseindex}`"
+if [ ! -x "$GLIMPSEINDEX" ]; then
+	echo 'glimpseindex can be obtained at https://github.com/mcgrof/glimpse.git'
+	exit 1
+fi
+
+find $DIR/* -name "*.[ch]" | $GLIMPSEINDEX -o -H . -F
Signature · 1 line
2.8.2
#6 lore raw

Re: [Cocci] [PATCH 3/4] scripts: add glimpse.sh for indexing the kernel

Quoted 3 lines

Glimpse is a tool you can use to index the kernel. The tool was recently open sourced under the ISC license and can be obtained at:

How do you think about to mention the script addition also directly in the commit message?

scripts/glimpse.sh line 4

Quoted 5 lines
scripts/glimpse.sh new file +4 -0
@@ -0,0 +1,12 @@
+#!/bin/bash
+
+DIR=$(dirname $(readlink -f $0))
+DIR="${DIR}/../"

Would you like to use the following variable assignment (instead of two before)?

+DIR="$(dirname $(readlink -f $0))/../"

By the way: How are the chances to achieve further software improvements?
https://github.com/gvelez17/glimpse/issues

Regards,
Markus

#7 lore raw

PATCH 4/4 coccicheck: add indexing enhancement options

In reply to Luis R. Rodriguez

Enable indexing optimizations heuristics. Coccinelle has support to make use of its own enhanced "grep" mechanisms instead of using regular grep for searching code 'coccigrep', in practice though this seems to not perform better than regular grep however its expected to help with some use cases so we use that if you have no other indexing options in place available.

Since git has its own index, support for using 'git grep' has been added to Coccinelle, that should on average perform better than using the internal cocci grep, and regular grep. Lastly, Coccinelle has had support for glimpseindex for a long while, however the tool was previously closed source, its now open sourced, and provides the best performance, so support that if we can detect you have a glimpse index.

These tests have been run on an 8 core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report

Before this patch with no indexing or anything:

real    16m22.435s
user    128m30.060s
sys     0m2.712s

Using coccigrep (after this patch if you have no .git):

real    16m27.650s
user    128m47.904s
sys     0m2.176s

If you have .git and therefore use gitgrep:

real    16m21.220s
user    129m30.940s
sys     0m2.060s

And if you have a .glimpse_index:

real    16m14.794s
user    128m42.356s
sys     0m1.880s
---
 scripts/coccicheck | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
scripts/coccicheck +24 -0
index eeb5fdc142ca..f31c9a152559 100755
@@ -5,6 +5,8 @@
 # version 1.0.0-rc11.
 #
 
+DIR=$(dirname $(readlink -f $0))
+DIR="${DIR}/../"
 SPATCH="`which ${SPATCH:=spatch}`"
 
 if [ ! -x "$SPATCH" ]; then
@@ -15,6 +17,20 @@ fi
 USE_JOBS="no"
 $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes"
 
+# 0. --use-glimpse currently outperforms all. Refer
+#    to scripts/glimpse.sh for details.
+# 1. Second best is --use-gitgrep, this is very comparable to --use-glimpse
+# 2. Use --use-coccigrep if no indexing options are available and your
+#    version of coccinelle supports it
+USE_GLIMPSE="no"
+$SPATCH --help | grep "\-\-use\-glimpse" > /dev/null && [ -f $DIR/.glimpse_index ] && USE_GLIMPSE="yes"
+
+USE_GITGREP="no"
+$SPATCH --help | grep "\-\-use\-gitgrep" > /dev/null && [ -d $DIR/.git ] && USE_GITGREP="yes"
+
+USE_COCCIGREP="no"
+$SPATCH --help | grep "\-\-use\-coccigrep" > /dev/null && USE_COCCIGREP="yes"
+
 # The verbosity may be set by the environmental parameter V=
 # as for example with 'make V=1 coccicheck'
 
@@ -89,6 +105,14 @@ else
 	OPTIONS="$OPTIONS --jobs $NPROC --chunksize 1"
 fi
 
+if [ "$USE_GLIMPSE" = "yes" ]; then
+	OPTIONS="$OPTIONS --use-glimpse"
+elif [ "$USE_GITGREP" = "yes" ]; then
+	OPTIONS="$OPTIONS --use-gitgrep"
+elif [ "$USE_COCCIGREP" = "yes" ]; then
+	OPTIONS="$OPTIONS --use-coccigrep"
+fi
+
 run_cmd_paramap() {
 	if [ $VERBOSE -ne 0 ] ; then
 		echo "Running ($NPROC in parallel): $@"
Signature · 1 line
2.8.2
#8 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 41 lines · Luis R. Rodriguez

Enable indexing optimizations heuristics. Coccinelle has support to make use of its own enhanced "grep" mechanisms instead of using regular grep for searching code 'coccigrep', in practice though this seems to not perform better than regular grep however its expected to help with some use cases so we use that if you have no other indexing options in place available.

Since git has its own index, support for using 'git grep' has been added to Coccinelle, that should on average perform better than using the internal cocci grep, and regular grep. Lastly, Coccinelle has had support for glimpseindex for a long while, however the tool was previously closed source, its now open sourced, and provides the best performance, so support that if we can detect you have a glimpse index.

These tests have been run on an 8 core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report

Before this patch with no indexing or anything:

real    16m22.435s
user    128m30.060s
sys     0m2.712s

Using coccigrep (after this patch if you have no .git):

real    16m27.650s
user    128m47.904s
sys     0m2.176s

If you have .git and therefore use gitgrep:

real    16m21.220s
user    129m30.940s
sys     0m2.060s
Quoted 5 lines

And if you have a .glimpse_index:

real    16m14.794s
user    128m42.356s
sys     0m1.880s

I don't see any convincing differences in these times.

I believe that Coccinelle's internal grep is always used, even with no option.

I'm puzzled why glimpse gives no benefit.

julia

Quoted 59 lines
---
 scripts/coccicheck | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
scripts/coccicheck +24 -0
index eeb5fdc142ca..f31c9a152559 100755
@@ -5,6 +5,8 @@
 # version 1.0.0-rc11.
 #
 
+DIR=$(dirname $(readlink -f $0))
+DIR="${DIR}/../"
 SPATCH="`which ${SPATCH:=spatch}`"
 
 if [ ! -x "$SPATCH" ]; then
@@ -15,6 +17,20 @@ fi
 USE_JOBS="no"
 $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes"
 
+# 0. --use-glimpse currently outperforms all. Refer
+#    to scripts/glimpse.sh for details.
+# 1. Second best is --use-gitgrep, this is very comparable to --use-glimpse
+# 2. Use --use-coccigrep if no indexing options are available and your
+#    version of coccinelle supports it
+USE_GLIMPSE="no"
+$SPATCH --help | grep "\-\-use\-glimpse" > /dev/null && [ -f $DIR/.glimpse_index ] && USE_GLIMPSE="yes"
+
+USE_GITGREP="no"
+$SPATCH --help | grep "\-\-use\-gitgrep" > /dev/null && [ -d $DIR/.git ] && USE_GITGREP="yes"
+
+USE_COCCIGREP="no"
+$SPATCH --help | grep "\-\-use\-coccigrep" > /dev/null && USE_COCCIGREP="yes"
+
 # The verbosity may be set by the environmental parameter V=
 # as for example with 'make V=1 coccicheck'
 
@@ -89,6 +105,14 @@ else
 	OPTIONS="$OPTIONS --jobs $NPROC --chunksize 1"
 fi
 
+if [ "$USE_GLIMPSE" = "yes" ]; then
+	OPTIONS="$OPTIONS --use-glimpse"
+elif [ "$USE_GITGREP" = "yes" ]; then
+	OPTIONS="$OPTIONS --use-gitgrep"
+elif [ "$USE_COCCIGREP" = "yes" ]; then
+	OPTIONS="$OPTIONS --use-coccigrep"
+fi
+
 run_cmd_paramap() {
 	if [ $VERBOSE -ne 0 ] ; then
 		echo "Running ($NPROC in parallel): $@"
Signature · 1 line
2.8.2
#9 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 47 lines · Julia Lawall
Quoted 43 lines · Luis R. Rodriguez

Enable indexing optimizations heuristics. Coccinelle has support to make use of its own enhanced "grep" mechanisms instead of using regular grep for searching code 'coccigrep', in practice though this seems to not perform better than regular grep however its expected to help with some use cases so we use that if you have no other indexing options in place available.

Since git has its own index, support for using 'git grep' has been added to Coccinelle, that should on average perform better than using the internal cocci grep, and regular grep. Lastly, Coccinelle has had support for glimpseindex for a long while, however the tool was previously closed source, its now open sourced, and provides the best performance, so support that if we can detect you have a glimpse index.

These tests have been run on an 8 core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report

Before this patch with no indexing or anything:

real    16m22.435s
user    128m30.060s
sys     0m2.712s

Using coccigrep (after this patch if you have no .git):

real    16m27.650s
user    128m47.904s
sys     0m2.176s

If you have .git and therefore use gitgrep:

real    16m21.220s
user    129m30.940s
sys     0m2.060s

And if you have a .glimpse_index:

Quoted 8 lines
Quoted 3 lines
real    16m14.794s
user    128m42.356s
sys     0m1.880s

I don't see any convincing differences in these times.

I believe that Coccinelle's internal grep is always used, even with no option.

Ah that would explain it. This uses coccinelle 1.0.5, is the default there to use --use-coccigrep if no other index is specified ?

Quoted 1 line

I'm puzzled why glimpse gives no benefit.

Well, slightly better.

Luis

#10 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 52 lines · Luis R. Rodriguez
Quoted 51 lines · Julia Lawall
Quoted 46 lines · Luis R. Rodriguez

Enable indexing optimizations heuristics. Coccinelle has support to make use of its own enhanced "grep" mechanisms instead of using regular grep for searching code 'coccigrep', in practice though this seems to not perform better than regular grep however its expected to help with some use cases so we use that if you have no other indexing options in place available.

Since git has its own index, support for using 'git grep' has been added to Coccinelle, that should on average perform better than using the internal cocci grep, and regular grep. Lastly, Coccinelle has had support for glimpseindex for a long while, however the tool was previously closed source, its now open sourced, and provides the best performance, so support that if we can detect you have a glimpse index.

These tests have been run on an 8 core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report

Before this patch with no indexing or anything:

real    16m22.435s
user    128m30.060s
sys     0m2.712s

Using coccigrep (after this patch if you have no .git):

real    16m27.650s
user    128m47.904s
sys     0m2.176s

If you have .git and therefore use gitgrep:

real    16m21.220s
user    129m30.940s
sys     0m2.060s

And if you have a .glimpse_index:

real    16m14.794s
user    128m42.356s
sys     0m1.880s
Quoted 7 lines
Quoted 4 lines

I don't see any convincing differences in these times.

I believe that Coccinelle's internal grep is always used, even with no option.

Ah that would explain it. This uses coccinelle 1.0.5, is the default there to use --use-coccigrep if no other index is specified ?

It has been the default for a long time.

Quoted 3 lines
Quoted 1 line

I'm puzzled why glimpse gives no benefit.

Well, slightly better.

No, it should be much better. You would have to look at the standard error to see if you are getting any benefit. There should be very few occurrences of Skipping if you are really using glimpse. In any case, if you asked for glimpse and it was not able to provide it, there should be warning messages at the top of stderr.

julia

#11 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 3 lines
Quoted 1 line

Well, slightly better.

No, it should be much better. You would have to look at the standard

I use id-utils regularly and it is indeed at least a magnitude better. The indexing often pays off already with the first coccinelle run for me. Highly recommended.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160610/81592df8/attachment.asc>

#12 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 7 lines · Wolfram Sang
Quoted 3 lines
Quoted 1 line

Well, slightly better.

No, it should be much better. You would have to look at the standard

I use id-utils regularly and it is indeed at least a magnitude better. The indexing often pays off already with the first coccinelle run for me. Highly recommended.

AFAICT coccinelle does not have integration support for id-utils though.

Luis

#13 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 1 line

AFAICT coccinelle does not have integration support for id-utils though.

I used it just today ;) -- "--use-idutils ./ID"

ID was generated with simple 'mkid -s'.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160610/7ed85c05/attachment.asc>

#14 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 6 lines · Wolfram Sang
Quoted 1 line

AFAICT coccinelle does not have integration support for id-utils though.

I used it just today ;) -- "--use-idutils ./ID"

ID was generated with simple 'mkid -s'.

Sweet, testing that now.

Luis

#15 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 9 lines · Luis R. Rodriguez
Quoted 6 lines · Wolfram Sang
Quoted 1 line

AFAICT coccinelle does not have integration support for id-utils though.

I used it just today ;) -- "--use-idutils ./ID"

ID was generated with simple 'mkid -s'.

Sweet, testing that now.

Boooyah :D

real    16m11.692s
user    127m50.388s
sys     0m2.168s

Luis

#17 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 6 lines · Wolfram Sang
Quoted 4 lines
real    16m11.692s
user    127m50.388s
sys     0m2.168s

That's better but not a magnitude, I wonder.

I think that it is because the filtering that Coccinelle does already works pretty well, and there are quite a lot of files (7514) that contain kfree.

julia

#18 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 3 lines

works pretty well, and there are quite a lot of files (7514) that contain kfree.

Ah, kfree. That explains, I missed that info.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160611/3a1b0fc4/attachment.asc>

#19 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

In reply to Julia Lawall
Quoted 9 lines · Julia Lawall
Quoted 5 lines · Wolfram Sang
Quoted 4 lines
real    16m11.692s
user    127m50.388s
sys     0m2.168s
Quoted 5 lines
Quoted 1 line

That's better but not a magnitude, I wonder.

I think that it is because the filtering that Coccinelle does already works pretty well, and there are quite a lot of files (7514) that contain kfree.

Is there another scripts/coccinelle/ file I can use to test against to demo against glimpse/idutils/gitgrep best?

Luis

#20 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 3 lines

Is there another scripts/coccinelle/ file I can use to test against to demo against glimpse/idutils/gitgrep best?

I'd think this one may be a candidate:

scripts/coccinelle/misc/irqf_oneshot.cocci

Not too many, but quite some matches over the tree.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160613/94028d8c/attachment.asc>

#21 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 9 lines · Wolfram Sang
Quoted 3 lines

Is there another scripts/coccinelle/ file I can use to test against to demo against glimpse/idutils/gitgrep best?

I'd think this one may be a candidate:

scripts/coccinelle/misc/irqf_oneshot.cocci

Not too many, but quite some matches over the tree.

Seems like a reasonable choice.

julia

#22 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 8 lines · Julia Lawall
Quoted 4 lines · Wolfram Sang
Quoted 3 lines

Is there another scripts/coccinelle/ file I can use to test against to demo against glimpse/idutils/gitgrep best?

Quoted 7 lines
Quoted 5 lines

I'd think this one may be a candidate:

scripts/coccinelle/misc/irqf_oneshot.cocci

Not too many, but quite some matches over the tree.

Seems like a reasonable choice.

With this one on a 32-core system, I get:

glimpse:

real    0m6.549s
user    0m49.136s
sys     0m3.076s

idutils:

real    0m6.749s
user    0m51.936s
sys     0m3.876s

gitgrep:

real    0m6.805s
user    0m51.572s
sys     0m4.432s

coccigrep:

real    0m16.369s
user    0m58.712s
sys     0m5.064s

I redirected stderr to stdout, and verified glimpse output has:

glimpse request = request_threaded_irq

Does this match expectations?

Luis

#23 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 39 lines · Luis R. Rodriguez
Quoted 8 lines · Julia Lawall
Quoted 4 lines · Wolfram Sang
Quoted 3 lines

Is there another scripts/coccinelle/ file I can use to test against to demo against glimpse/idutils/gitgrep best?

Quoted 7 lines
Quoted 5 lines

I'd think this one may be a candidate:

scripts/coccinelle/misc/irqf_oneshot.cocci

Not too many, but quite some matches over the tree.

Seems like a reasonable choice.

With this one on a 32-core system, I get:

glimpse:

real    0m6.549s
user    0m49.136s
sys     0m3.076s

idutils:

real    0m6.749s
user    0m51.936s
sys     0m3.876s

gitgrep:

real    0m6.805s
user    0m51.572s
sys     0m4.432s

coccigrep:

real    0m16.369s
user    0m58.712s
sys     0m5.064s
Quoted 5 lines

I redirected stderr to stdout, and verified glimpse output has:

glimpse request = request_threaded_irq

Does this match expectations?

Yes. I'm not sure that gitgrep would work as well when there are multiple keywords. It may descend to coccigrep.

julia

#24 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

In reply to Luis R. Rodriguez
Quoted 9 lines · Luis R. Rodriguez
Quoted 6 lines · Wolfram Sang
Quoted 1 line

AFAICT coccinelle does not have integration support for id-utils though.

I used it just today ;) -- "--use-idutils ./ID"

ID was generated with simple 'mkid -s'.

Sweet, testing that now.

It's not as efficient as glimpse because the query language is simpler. So more filtering has to be done at the ocaml level. But it's probably fine in most cases.

julia

#25 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 2 lines

It's not as efficient as glimpse because the query language is simpler.

Interesting, what is missing compared to glimpse?

Quoted 2 lines

So more filtering has to be done at the ocaml level. But it's probably fine in most cases.

For me, it has two advantages over glimpse:

a) it is in the debian package repository
b) the same database can be used with the code browser 'seascope' which can do nice things by feeding ctags on the fly with data from id-utils.

Mileages vary, of course, just wanted to mention it to give pointers.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20160611/9398ddd1/attachment-0001.asc>

#26 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 4 lines · Wolfram Sang
Quoted 2 lines

It's not as efficient as glimpse because the query language is simpler.

Interesting, what is missing compared to glimpse?

Glimpse allows queries that are arbitrary formulas, up to a limited level of complexity, involving both && and ||. For idutils, Coccinelle runs lid on each of the tokens in the formula, and then does unions and intersections on the result.

julia

Quoted 13 lines
Quoted 2 lines

So more filtering has to be done at the ocaml level. But it's probably fine in most cases.

For me, it has two advantages over glimpse:

a) it is in the debian package repository
b) the same database can be used with the code browser 'seascope' which can do nice things by feeding ctags on the fly with data from id-utils.

Mileages vary, of course, just wanted to mention it to give pointers.

#27 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

In reply to Wolfram Sang
Quoted 5 lines · Wolfram Sang
Quoted 1 line

AFAICT coccinelle does not have integration support for id-utils though.

I used it just today ;) -- "--use-idutils ./ID"

ID was generated with simple 'mkid -s'.

Coccinelle includes a script scripts/idutils_index.sh

This does mkid -i C --output .id-utils.index *

julia

#28 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 6 lines · Julia Lawall
Quoted 2 lines · Wolfram Sang
Quoted 1 line

AFAICT coccinelle does not have integration support for id-utils though.

Quoted 7 lines
Quoted 3 lines

I used it just today ;) -- "--use-idutils ./ID"

ID was generated with simple 'mkid -s'.

Coccinelle includes a script scripts/idutils_index.sh

This does mkid -i C --output .id-utils.index *

I'll add support for detecting both. An issue of course is if any of these indexes grows stale. So I'll advise against these and recommend gitgrep unless the user has a hook to update index regularly.

Luis

#29 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

In reply to Luis R. Rodriguez
Quoted 10 lines · Luis R. Rodriguez
Quoted 7 lines · Wolfram Sang
Quoted 3 lines
Quoted 1 line

Well, slightly better.

No, it should be much better. You would have to look at the standard

I use id-utils regularly and it is indeed at least a magnitude better. The indexing often pays off already with the first coccinelle run for me. Highly recommended.

AFAICT coccinelle does not have integration support for id-utils though.

Yes it does, for years.

julia

#30 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

In reply to Julia Lawall
Quoted 68 lines · Julia Lawall
Quoted 52 lines · Luis R. Rodriguez
Quoted 51 lines · Julia Lawall
Quoted 46 lines · Luis R. Rodriguez

Enable indexing optimizations heuristics. Coccinelle has support to make use of its own enhanced "grep" mechanisms instead of using regular grep for searching code 'coccigrep', in practice though this seems to not perform better than regular grep however its expected to help with some use cases so we use that if you have no other indexing options in place available.

Since git has its own index, support for using 'git grep' has been added to Coccinelle, that should on average perform better than using the internal cocci grep, and regular grep. Lastly, Coccinelle has had support for glimpseindex for a long while, however the tool was previously closed source, its now open sourced, and provides the best performance, so support that if we can detect you have a glimpse index.

These tests have been run on an 8 core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report

Before this patch with no indexing or anything:

real    16m22.435s
user    128m30.060s
sys     0m2.712s

Using coccigrep (after this patch if you have no .git):

real    16m27.650s
user    128m47.904s
sys     0m2.176s

If you have .git and therefore use gitgrep:

real    16m21.220s
user    129m30.940s
sys     0m2.060s

And if you have a .glimpse_index:

real    16m14.794s
user    128m42.356s
sys     0m1.880s
Quoted 7 lines
Quoted 4 lines

I don't see any convincing differences in these times.

I believe that Coccinelle's internal grep is always used, even with no option.

Ah that would explain it. This uses coccinelle 1.0.5, is the default there to use --use-coccigrep if no other index is specified ?

It has been the default for a long time.

Quoted 2 lines
Quoted 1 line

I'm puzzled why glimpse gives no benefit.

Quoted 7 lines
Quoted 1 line

Well, slightly better.

No, it should be much better. You would have to look at the standard error to see if you are getting any benefit. There should be very few occurrences of Skipping if you are really using glimpse. In any case, if you asked for glimpse and it was not able to provide it, there should be warning messages at the top of stderr.

I'll redirect stderr to stdout by default when parmap support is used then.

Luis

#31 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 71 lines · Luis R. Rodriguez
Quoted 70 lines · Julia Lawall
Quoted 52 lines · Luis R. Rodriguez
Quoted 51 lines · Julia Lawall
Quoted 46 lines · Luis R. Rodriguez

Enable indexing optimizations heuristics. Coccinelle has support to make use of its own enhanced "grep" mechanisms instead of using regular grep for searching code 'coccigrep', in practice though this seems to not perform better than regular grep however its expected to help with some use cases so we use that if you have no other indexing options in place available.

Since git has its own index, support for using 'git grep' has been added to Coccinelle, that should on average perform better than using the internal cocci grep, and regular grep. Lastly, Coccinelle has had support for glimpseindex for a long while, however the tool was previously closed source, its now open sourced, and provides the best performance, so support that if we can detect you have a glimpse index.

These tests have been run on an 8 core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report

Before this patch with no indexing or anything:

real    16m22.435s
user    128m30.060s
sys     0m2.712s

Using coccigrep (after this patch if you have no .git):

real    16m27.650s
user    128m47.904s
sys     0m2.176s

If you have .git and therefore use gitgrep:

real    16m21.220s
user    129m30.940s
sys     0m2.060s

And if you have a .glimpse_index:

real    16m14.794s
user    128m42.356s
sys     0m1.880s
Quoted 7 lines
Quoted 4 lines

I don't see any convincing differences in these times.

I believe that Coccinelle's internal grep is always used, even with no option.

Ah that would explain it. This uses coccinelle 1.0.5, is the default there to use --use-coccigrep if no other index is specified ?

It has been the default for a long time.

Quoted 3 lines
Quoted 1 line

I'm puzzled why glimpse gives no benefit.

Well, slightly better.

Quoted 7 lines
Quoted 5 lines

No, it should be much better. You would have to look at the standard error to see if you are getting any benefit. There should be very few occurrences of Skipping if you are really using glimpse. In any case, if you asked for glimpse and it was not able to provide it, there should be warning messages at the top of stderr.

I'll redirect stderr to stdout by default when parmap support is used then.

Usually I put them in different files.

julia

#32 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 81 lines · Julia Lawall
Quoted 77 lines · Luis R. Rodriguez
Quoted 75 lines · Julia Lawall
Quoted 52 lines · Luis R. Rodriguez
Quoted 51 lines · Julia Lawall
Quoted 46 lines · Luis R. Rodriguez

Enable indexing optimizations heuristics. Coccinelle has support to make use of its own enhanced "grep" mechanisms instead of using regular grep for searching code 'coccigrep', in practice though this seems to not perform better than regular grep however its expected to help with some use cases so we use that if you have no other indexing options in place available.

Since git has its own index, support for using 'git grep' has been added to Coccinelle, that should on average perform better than using the internal cocci grep, and regular grep. Lastly, Coccinelle has had support for glimpseindex for a long while, however the tool was previously closed source, its now open sourced, and provides the best performance, so support that if we can detect you have a glimpse index.

These tests have been run on an 8 core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report

Before this patch with no indexing or anything:

real    16m22.435s
user    128m30.060s
sys     0m2.712s

Using coccigrep (after this patch if you have no .git):

real    16m27.650s
user    128m47.904s
sys     0m2.176s

If you have .git and therefore use gitgrep:

real    16m21.220s
user    129m30.940s
sys     0m2.060s

And if you have a .glimpse_index:

real    16m14.794s
user    128m42.356s
sys     0m1.880s
Quoted 7 lines
Quoted 4 lines

I don't see any convincing differences in these times.

I believe that Coccinelle's internal grep is always used, even with no option.

Ah that would explain it. This uses coccinelle 1.0.5, is the default there to use --use-coccigrep if no other index is specified ?

It has been the default for a long time.

Quoted 3 lines
Quoted 1 line

I'm puzzled why glimpse gives no benefit.

Well, slightly better.

No, it should be much better. You would have to look at the standard error to see if you are getting any benefit. There should be very few occurrences of Skipping if you are really using glimpse. In any case, if you asked for glimpse and it was not able to provide it, there should be warning messages at the top of stderr.

Quoted 3 lines
Quoted 1 line

I'll redirect stderr to stdout by default when parmap support is used then.

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

Luis

#33 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 82 lines · Luis R. Rodriguez
Quoted 81 lines · Julia Lawall
Quoted 77 lines · Luis R. Rodriguez
Quoted 75 lines · Julia Lawall
Quoted 52 lines · Luis R. Rodriguez
Quoted 51 lines · Julia Lawall
Quoted 46 lines · Luis R. Rodriguez

Enable indexing optimizations heuristics. Coccinelle has support to make use of its own enhanced "grep" mechanisms instead of using regular grep for searching code 'coccigrep', in practice though this seems to not perform better than regular grep however its expected to help with some use cases so we use that if you have no other indexing options in place available.

Since git has its own index, support for using 'git grep' has been added to Coccinelle, that should on average perform better than using the internal cocci grep, and regular grep. Lastly, Coccinelle has had support for glimpseindex for a long while, however the tool was previously closed source, its now open sourced, and provides the best performance, so support that if we can detect you have a glimpse index.

These tests have been run on an 8 core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report

Before this patch with no indexing or anything:

real    16m22.435s
user    128m30.060s
sys     0m2.712s

Using coccigrep (after this patch if you have no .git):

real    16m27.650s
user    128m47.904s
sys     0m2.176s

If you have .git and therefore use gitgrep:

real    16m21.220s
user    129m30.940s
sys     0m2.060s

And if you have a .glimpse_index:

real    16m14.794s
user    128m42.356s
sys     0m1.880s
Quoted 7 lines
Quoted 4 lines

I don't see any convincing differences in these times.

I believe that Coccinelle's internal grep is always used, even with no option.

Ah that would explain it. This uses coccinelle 1.0.5, is the default there to use --use-coccigrep if no other index is specified ?

It has been the default for a long time.

Quoted 3 lines
Quoted 1 line

I'm puzzled why glimpse gives no benefit.

Well, slightly better.

No, it should be much better. You would have to look at the standard error to see if you are getting any benefit. There should be very few occurrences of Skipping if you are really using glimpse. In any case, if you asked for glimpse and it was not able to provide it, there should be warning messages at the top of stderr.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

I'll redirect stderr to stdout by default when parmap support is used then.

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

I don't understand the connection with parmap.

Originally our use of parmap made output files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

julia

#34 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 11 lines · Julia Lawall
Quoted 7 lines · Luis R. Rodriguez
Quoted 6 lines · Julia Lawall
Quoted 2 lines · Luis R. Rodriguez

I'll redirect stderr to stdout by default when parmap support is used then.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

I don't understand the connection with parmap.

When parmap support is not available the cocciscript will currently disregard stderr, output is provided as it comes to stdout from each thread I guess.

Quoted 5 lines

Originally our use of parmap made output files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

I'll use ${DIR}/coccicheck.$$.err for stderr.

Luis

#35 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 18 lines · Luis R. Rodriguez
Quoted 17 lines · Julia Lawall
Quoted 12 lines · Luis R. Rodriguez
Quoted 7 lines · Julia Lawall
Quoted 2 lines · Luis R. Rodriguez

I'll redirect stderr to stdout by default when parmap support is used then.

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

Quoted 5 lines
Quoted 1 line

I don't understand the connection with parmap.

When parmap support is not available the cocciscript will currently disregard stderr, output is provided as it comes to stdout from each thread I guess.

Deepa's recent patch to coccicheck made apparent that Coccicheck uses --very-quiet, so there is standard error.

Quoted 7 lines
Quoted 5 lines

Originally our use of parmap made output files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

I'll use ${DIR}/coccicheck.$$.err for stderr.

What is ${DIR}? and what is $$?

julia

#36 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 22 lines · Julia Lawall
Quoted 18 lines · Luis R. Rodriguez
Quoted 17 lines · Julia Lawall
Quoted 12 lines · Luis R. Rodriguez
Quoted 7 lines · Julia Lawall
Quoted 2 lines · Luis R. Rodriguez

I'll redirect stderr to stdout by default when parmap support is used then.

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

I don't understand the connection with parmap.

When parmap support is not available the cocciscript will currently disregard stderr, output is provided as it comes to stdout from each thread I guess.

Deepa's recent patch to coccicheck made apparent that Coccicheck uses --very-quiet, so there is standard error.

OK I'm disegarding the redirect for non-parmap for now but we'd have to determine if we want to append or add one per PID... I rather leave that stuff as-is and encourage folks to upgrade coccinelle.

Quoted 9 lines
Quoted 7 lines
Quoted 5 lines

Originally our use of parmap made output files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

I'll use ${DIR}/coccicheck.$$.err for stderr.

What is ${DIR}? and what is $$?

When you run scripts/coccicheck we take the absolute directory of it and then go down one level of directory, so in this case it would be the base directory of the Linux kernel.

$$ is the PID of the bash script.

Luis

#37 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 29 lines · Luis R. Rodriguez
Quoted 28 lines · Julia Lawall
Quoted 23 lines · Luis R. Rodriguez
Quoted 11 lines · Julia Lawall
Quoted 7 lines · Luis R. Rodriguez
Quoted 6 lines · Julia Lawall
Quoted 2 lines · Luis R. Rodriguez

I'll redirect stderr to stdout by default when parmap support is used then.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

I don't understand the connection with parmap.

When parmap support is not available the cocciscript will currently disregard stderr, output is provided as it comes to stdout from each thread I guess.

Quoted 6 lines
Quoted 2 lines

Deepa's recent patch to coccicheck made apparent that Coccicheck uses --very-quiet, so there is standard error.

OK I'm disegarding the redirect for non-parmap for now but we'd have to determine if we want to append or add one per PID... I rather leave that stuff as-is and encourage folks to upgrade coccinelle.

If coccicheck is using --very-quiet, there will not be much stderr of interest when using parmap either.

I'm not sure to understan the issue about appending. Appending what to what? While Coccinelle is running, you have a directory with the same name as your semantic patch (or a directory name that you specify with --tmp-dir) that has separate files for each core's standard output and standard error. At the end, when all the code has been treated, Coccinelle writes the successive stdouts to standard output, and the sucessive stderrs to standard error.

Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines

Originally our use of parmap made output files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

I'll use ${DIR}/coccicheck.$$.err for stderr.

Quoted 7 lines
Quoted 1 line

What is ${DIR}? and what is $$?

When you run scripts/coccicheck we take the absolute directory of it and then go down one level of directory, so in this case it would be the base directory of the Linux kernel.

$$ is the PID of the bash script.

OK. I still don't find PIDs useful, but I guess if we are talking about the entire output of coccicheck, there is not much else to do. Normally, I don't want these files accumulating, and just write over the old ones.

julia

#38 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 36 lines · Julia Lawall
Quoted 32 lines · Luis R. Rodriguez
Quoted 30 lines · Julia Lawall
Quoted 18 lines · Luis R. Rodriguez
Quoted 17 lines · Julia Lawall
Quoted 12 lines · Luis R. Rodriguez
Quoted 7 lines · Julia Lawall
Quoted 2 lines · Luis R. Rodriguez

I'll redirect stderr to stdout by default when parmap support is used then.

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

Quoted 5 lines
Quoted 1 line

I don't understand the connection with parmap.

When parmap support is not available the cocciscript will currently disregard stderr, output is provided as it comes to stdout from each thread I guess.

Deepa's recent patch to coccicheck made apparent that Coccicheck uses --very-quiet, so there is standard error.

Quoted 6 lines
Quoted 3 lines

OK I'm disegarding the redirect for non-parmap for now but we'd have to determine if we want to append or add one per PID... I rather leave that stuff as-is and encourage folks to upgrade coccinelle.

If coccicheck is using --very-quiet, there will not be much stderr of interest when using parmap either.

OK I don't follow. Does coccinelle only direct error to stderr when --very-quiet is used ? Or does using --very-quiet suppress stderr ?

Quoted 8 lines

I'm not sure to understan the issue about appending. Appending what to what?
While Coccinelle is running, you have a directory with the same name as your semantic patch (or a directory name that you specify with --tmp-dir) that has separate files for each core's standard output and standard error. At the end, when all the code has been treated, Coccinelle writes the successive stdouts to standard output, and the sucessive stderrs to standard error.

Ah I see. OK yeah never mind about appending.

Quoted 14 lines
Quoted 14 lines
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines

Originally our use of parmap made output, specia files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

I'll use ${DIR}/coccicheck.$$.err for stderr.

What is ${DIR}? and what is $$?

When you run scripts/coccicheck we take the absolute directory of it and then go down one level of directory, so in this case it would be the base directory of the Linux kernel.

Quoted 5 lines
Quoted 1 line

$$ is the PID of the bash script.

OK. I still don't find PIDs useful, but I guess if we are talking about the entire output of coccicheck, there is not much else to do. Normally, I don't want these files accumulating, and just write over the old ones.

Which is why I would much prefer to instead just redirect in coccicheck case stderr to stdout from coccinelle. Is that preferred?

Luis

#39 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 41 lines · Luis R. Rodriguez
Quoted 40 lines · Julia Lawall
Quoted 35 lines · Luis R. Rodriguez
Quoted 22 lines · Julia Lawall
Quoted 18 lines · Luis R. Rodriguez
Quoted 17 lines · Julia Lawall
Quoted 12 lines · Luis R. Rodriguez
Quoted 7 lines · Julia Lawall
Quoted 2 lines · Luis R. Rodriguez

I'll redirect stderr to stdout by default when parmap support is used then.

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

I don't understand the connection with parmap.

When parmap support is not available the cocciscript will currently disregard stderr, output is provided as it comes to stdout from each thread I guess.

Deepa's recent patch to coccicheck made apparent that Coccicheck uses --very-quiet, so there is standard error.

OK I'm disegarding the redirect for non-parmap for now but we'd have to determine if we want to append or add one per PID... I rather leave that stuff as-is and encourage folks to upgrade coccinelle.

Quoted 5 lines
Quoted 2 lines

If coccicheck is using --very-quiet, there will not be much stderr of interest when using parmap either.

OK I don't follow. Does coccinelle only direct error to stderr when --very-quiet is used ? Or does using --very-quiet suppress stderr ?

--very-quiet suppresses most administrative messages that go to stderr. There are still actual errors. Bu you don't see eg what file is being currently processed.

Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines

Originally our use of parmap made output, specia files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

I'll use ${DIR}/coccicheck.$$.err for stderr.

What is ${DIR}? and what is $$?

When you run scripts/coccicheck we take the absolute directory of it and then go down one level of directory, so in this case it would be the base directory of the Linux kernel.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

$$ is the PID of the bash script.

OK. I still don't find PIDs useful, but I guess if we are talking about the entire output of coccicheck, there is not much else to do. Normally, I don't want these files accumulating, and just write over the old ones.

Which is why I would much prefer to instead just redirect in coccicheck case stderr to stdout from coccinelle. Is that preferred?

Then things will be merged in strange ways.

Why not just let the user decide what to do with these things?

julia

#40 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 48 lines · Julia Lawall
Quoted 44 lines · Luis R. Rodriguez
Quoted 42 lines · Julia Lawall
Quoted 29 lines · Luis R. Rodriguez
Quoted 28 lines · Julia Lawall
Quoted 23 lines · Luis R. Rodriguez
Quoted 11 lines · Julia Lawall
Quoted 7 lines · Luis R. Rodriguez
Quoted 6 lines · Julia Lawall
Quoted 2 lines · Luis R. Rodriguez

I'll redirect stderr to stdout by default when parmap support is used then.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

I don't understand the connection with parmap.

When parmap support is not available the cocciscript will currently disregard stderr, output is provided as it comes to stdout from each thread I guess.

Quoted 6 lines
Quoted 2 lines

Deepa's recent patch to coccicheck made apparent that Coccicheck uses --very-quiet, so there is standard error.

OK I'm disegarding the redirect for non-parmap for now but we'd have to determine if we want to append or add one per PID... I rather leave that stuff as-is and encourage folks to upgrade coccinelle.

If coccicheck is using --very-quiet, there will not be much stderr of interest when using parmap either.

Quoted 6 lines
Quoted 2 lines

OK I don't follow. Does coccinelle only direct error to stderr when --very-quiet is used ? Or does using --very-quiet suppress stderr ?

--very-quiet suppresses most administrative messages that go to stderr. There are still actual errors. Bu you don't see eg what file is being currently processed.

OK thanks. I remove --very-quiet now if --profile is used within SPFLAGS, I'll extend this to also avoid --very-quiet if --show-trying is used. SPFLAGS is where you can specify extra options that the script doesn't specifically support.

Quoted 20 lines
Quoted 20 lines
Quoted 19 lines
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines

Originally our use of parmap made output, specia files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

I'll use ${DIR}/coccicheck.$$.err for stderr.

Quoted 7 lines
Quoted 1 line

What is ${DIR}? and what is $$?

When you run scripts/coccicheck we take the absolute directory of it and then go down one level of directory, so in this case it would be the base directory of the Linux kernel.

$$ is the PID of the bash script.

OK. I still don't find PIDs useful, but I guess if we are talking about the entire output of coccicheck, there is not much else to do. Normally, I don't want these files accumulating, and just write over the old ones.

Quoted 6 lines
Quoted 2 lines

Which is why I would much prefer to instead just redirect in coccicheck case stderr to stdout from coccinelle. Is that preferred?

Then things will be merged in strange ways.

Why not just let the user decide what to do with these things?

Sure, what should be the default?

Luis

#41 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 52 lines · Luis R. Rodriguez
Quoted 51 lines · Julia Lawall
Quoted 46 lines · Luis R. Rodriguez
Quoted 36 lines · Julia Lawall
Quoted 32 lines · Luis R. Rodriguez
Quoted 30 lines · Julia Lawall
Quoted 18 lines · Luis R. Rodriguez
Quoted 17 lines · Julia Lawall
Quoted 12 lines · Luis R. Rodriguez
Quoted 7 lines · Julia Lawall
Quoted 2 lines · Luis R. Rodriguez

I'll redirect stderr to stdout by default when parmap support is used then.

Usually I put them in different files.

We can do that as well but I would only want to deal with parmap support case. Any preference? How about .coccicheck.stderr.$PID where PID would be the PID of the shell script?

Quoted 5 lines
Quoted 1 line

I don't understand the connection with parmap.

When parmap support is not available the cocciscript will currently disregard stderr, output is provided as it comes to stdout from each thread I guess.

Deepa's recent patch to coccicheck made apparent that Coccicheck uses --very-quiet, so there is standard error.

Quoted 6 lines
Quoted 3 lines

OK I'm disegarding the redirect for non-parmap for now but we'd have to determine if we want to append or add one per PID... I rather leave that stuff as-is and encourage folks to upgrade coccinelle.

If coccicheck is using --very-quiet, there will not be much stderr of interest when using parmap either.

OK I don't follow. Does coccinelle only direct error to stderr when --very-quiet is used ? Or does using --very-quiet suppress stderr ?

Quoted 7 lines
Quoted 3 lines

--very-quiet suppresses most administrative messages that go to stderr. There are still actual errors. Bu you don't see eg what file is being currently processed.

OK thanks. I remove --very-quiet now if --profile is used within SPFLAGS, I'll extend this to also avoid --very-quiet if --show-trying is used. SPFLAGS is where you can specify extra options that the script doesn't specifically support.

If it is more convenient, you don't actually have to remove --very-quiet. You can just put --quiet before --show-trying or --profile. --quiet will override --very-quiet.

Quoted 20 lines
Quoted 20 lines
Quoted 20 lines
Quoted 19 lines
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines

Originally our use of parmap made output, specia files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

I'll use ${DIR}/coccicheck.$$.err for stderr.

Quoted 7 lines
Quoted 1 line

What is ${DIR}? and what is $$?

When you run scripts/coccicheck we take the absolute directory of it and then go down one level of directory, so in this case it would be the base directory of the Linux kernel.

$$ is the PID of the bash script.

OK. I still don't find PIDs useful, but I guess if we are talking about the entire output of coccicheck, there is not much else to do. Normally, I don't want these files accumulating, and just write over the old ones.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Which is why I would much prefer to instead just redirect in coccicheck case stderr to stdout from coccinelle. Is that preferred?

Then things will be merged in strange ways.

Why not just let the user decide what to do with these things?

Sure, what should be the default?

I would normally just expect standard output and standard error to appear randomly on the screen. That is, no management effort from the tool at all.

julia

#42 lore raw

Re: [PATCH 4/4] coccicheck: add indexing enhancement options

Quoted 10 lines · Julia Lawall
Quoted 3 lines · Luis R. Rodriguez

OK thanks. I remove --very-quiet now if --profile is used within SPFLAGS, I'll extend this to also avoid --very-quiet if --show-trying is used. SPFLAGS is where you can specify extra options that the script doesn't specifically support.

If it is more convenient, you don't actually have to remove --very-quiet. You can just put --quiet before --show-trying or --profile. --quiet will override --very-quiet.

How about just:

if [ "$SPFLAGS" == *"--profile"* -o "$SPFLAGS" == "--show-trying" ]; then
FLAGS="--quiet $SPFLAGS"
else
FLAGS="--very-quiet $SPFLAGS"
fi

Quoted 25 lines
Quoted 25 lines
Quoted 25 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines

Originally our use of parmap made output, specia files based on pids. Maybe this is the default for parmap. I found this completely unusable. I guess one could look at the dates to see which file is the most recent one, but it seems tedious. If you are putting the standard output in x.out, then put the standard error in x.err.

I'll use ${DIR}/coccicheck.$$.err for stderr.

What is ${DIR}? and what is $$?

When you run scripts/coccicheck we take the absolute directory of it and then go down one level of directory, so in this case it would be the base directory of the Linux kernel.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

$$ is the PID of the bash script.

OK. I still don't find PIDs useful, but I guess if we are talking about the entire output of coccicheck, there is not much else to do. Normally, I don't want these files accumulating, and just write over the old ones.

Which is why I would much prefer to instead just redirect in coccicheck case stderr to stdout from coccinelle. Is that preferred?

Then things will be merged in strange ways.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Why not just let the user decide what to do with these things?

Sure, what should be the default?

I would normally just expect standard output and standard error to appear randomly on the screen. That is, no management effort from the tool at all.

But the thing is, stderr is ignored now given that a shell script is used wrapped over a Makefile so if we want what you describe I think we do have to by default do 2>&1 on the spatch run command.

Luis

#43 lore raw

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

In reply to Luis R. Rodriguez
Quoted 4 lines

in practice though this seems to not perform better than regular grep however its expected to help with some use cases so we use that if you have no other indexing options in place available.

Would you like to fix a typo in this paragraph?

Regards,
Markus