8:32 p.m.
This shaves off 4 % of syntax-check runtime on my 8-core system,
which is enough of a reason for me.
Surely it saves more on our overloaded CI workers.
Ján Tomko (3):
scripts: speedup prohibit-duplicate-header
scripts: check-aclrules: use in instead of find
scripts: check-aclrules: use regular expressions less often
scripts/check-aclrules.py | 38 ++++++++++++++++++----------
scripts/prohibit-duplicate-header.py | 4 +++
2 files changed, 29 insertions(+), 13 deletions(-)
--
2.19.2
8:32 p.m.
New subject: [libvirt] [PATCH 1/3] scripts: speedup prohibit-duplicate-header
Running regular expressions with capture groups is expensive.
Bail out early if the line does not start with a '#'.
This reduces the runtime of the check by two thirds.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
scripts/prohibit-duplicate-header.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/scripts/prohibit-duplicate-header.py b/scripts/prohibit-duplicate-header.py
index dfdfa0bf0b..420311ccef 100644
--- a/scripts/prohibit-duplicate-header.py
+++ b/scripts/prohibit-duplicate-header.py
@@ -30,6 +30,10 @@ def check_file(filename):
for line in fh:
lineno = lineno + 1
+ # skip non-matching lines early
+ if line[0] != '#':
+ continue
+
headermatch = re.search(r'''^# *include
*[<"]([^>"]*\.h)[">]''', line)
if headermatch is not None:
inc = headermatch.group(1)
--
2.19.2
10:26 a.m.
New subject: [libvirt] [PATCH 1/3] scripts: speedup prohibit-duplicate-header
On Wed, Nov 20, 2019 at 07:32:44PM +0100, Ján Tomko wrote:
Running regular expressions with capture groups is expensive.
Bail out early if the line does not start with a '#'.
This reduces the runtime of the check by two thirds.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
8:32 p.m.
New subject: [libvirt] [PATCH 2/3] scripts: check-aclrules: use in instead of find
For checking whether a substring is present in a string,
using the pattern:
"str" in string
is slightly faster than:
string.find("str") != -1
Use it to shave off 4 % of the runtime of this script that
processes quite a few long source files.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
scripts/check-aclrules.py | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/scripts/check-aclrules.py b/scripts/check-aclrules.py
index e06a019b00..d145e59164 100755
--- a/scripts/check-aclrules.py
+++ b/scripts/check-aclrules.py
@@ -97,7 +97,7 @@ def fixup_name(name):
def name_to_ProcName(name):
elems = []
- if name.find("_") != -1 or name.lower() in ["open",
"close"]:
+ if "_" in name or name.lower() in ["open", "close"]:
elems = [n.lower().capitalize() for n in name.split("_")]
else:
elems = [name]
@@ -116,11 +116,11 @@ with open(proto, "r") as fh:
filtered = False
for line in fh:
- if line.find("/**") != -1:
+ if "/**" in line:
incomment = True
filtered = False
elif incomment:
- if line.find("* @aclfilter") != -1:
+ if "* @aclfilter" in line:
filtered = True
elif filtered:
m = re.search(r'''REMOTE_PROC_(.*)\s+=\s*\d+''',
line)
@@ -211,7 +211,7 @@ def process_file(filename):
# an ACL function
if intable:
assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''',
line)
- if line.find("}") != -1:
+ if "}" in line:
intable = False
table = None
elif assign is not None:
@@ -247,9 +247,9 @@ def process_file(filename):
intable = True
table = name
- if line.find("{") != -1:
+ if "{" in line:
brace = brace + 1
- if line.find("}") != -1:
+ if "}" in line:
brace = brace - 1
return errs
--
2.19.2
10:27 a.m.
New subject: [libvirt] [PATCH 2/3] scripts: check-aclrules: use in instead of find
On Wed, Nov 20, 2019 at 07:32:45PM +0100, Ján Tomko wrote:
For checking whether a substring is present in a string,
using the pattern:
"str" in string
is slightly faster than:
string.find("str") != -1
Use it to shave off 4 % of the runtime of this script that
processes quite a few long source files.
This is also the case in python FWIW...
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
11:20 a.m.
New subject: [libvirt] [PATCH 2/3] scripts: check-aclrules: use in instead of find
On Thu, Nov 21, 2019 at 09:27:31AM +0100, Erik Skultety wrote:
On Wed, Nov 20, 2019 at 07:32:45PM +0100, Ján Tomko wrote:
> For checking whether a substring is present in a string,
> using the pattern:
> "str" in string
> is slightly faster than:
> string.find("str") != -1
>
> Use it to shave off 4 % of the runtime of this script that
> processes quite a few long source files.
>
This is also the case in python FWIW...
Derp...clearly I didn't have my cup of coffee in the morning, since I thought
this was related to perl, sorry for the noise.
Erik
> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> ---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:32 p.m.
New subject: [libvirt] [PATCH 3/3] scripts: check-aclrules: use regular expressions less often
Use a simple
if "substr" in line
before running a regular expression, which saves time,
especially if the regex has a capture group.
This reduces runtime of the check by almost 78 % for me.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
scripts/check-aclrules.py | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/scripts/check-aclrules.py b/scripts/check-aclrules.py
index d145e59164..40c47c1c6b 100755
--- a/scripts/check-aclrules.py
+++ b/scripts/check-aclrules.py
@@ -150,13 +150,21 @@ def process_file(filename):
# Looks for anything which appears to be a function
# body name. Doesn't matter if we pick up bogus stuff
# here, as long as we don't miss valid stuff
- m = re.search(r'''\b(\w+)\(''', line)
+ m = None
+ if "(" in line:
+ m = re.search(r'''\b(\w+)\(''', line)
if m is not None:
maybefunc = m.group(1)
elif brace > 0:
- ensureacl = re.search(r'''(\w+)EnsureACL''',
line)
- checkacl = re.search(r'''(\w+)CheckACL''', line)
- stub = re.search(r'''\b(\w+)\(''', line)
+ ensureacl = None
+ checkacl = None
+ stub = None
+ if "EnsureACL" in line:
+ ensureacl = re.search(r'''(\w+)EnsureACL''',
line)
+ if "CheckACL" in line:
+ checkacl = re.search(r'''(\w+)CheckACL''',
line)
+ if "(" in line:
+ stub = re.search(r'''\b(\w+)\(''', line)
if ensureacl is not None:
# Record the fact that maybefunc contains an
# ACL call, and make sure it is the right call!
@@ -210,7 +218,9 @@ def process_file(filename):
# every func listed there, has an impl which calls
# an ACL function
if intable:
- assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''',
line)
+ assign = None
+ if '"' in line:
+ assign =
re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line)
if "}" in line:
intable = False
table = None
@@ -237,8 +247,10 @@ def process_file(filename):
file=sys.stderr)
errs = True
else:
- m =
re.search(r'''^(?:static\s+)?(vir(?:\w+)?Driver)\s+''',
- line)
+ m = None
+ if "Driver" in line:
+ m =
re.search(r'''^(?:static\s+)?(vir(?:\w+)?Driver)\s+''',
+ line)
if m is not None:
name = m.group(1)
if name not in ["virNWFilterCallbackDriver",
--
2.19.2
10 a.m.
New subject: [libvirt] [PATCH 3/3] scripts: check-aclrules: use regular expressions less often
On Wed, Nov 20, 2019 at 07:32:46PM +0100, Ján Tomko wrote:
Use a simple
if "substr" in line
before running a regular expression, which saves time,
especially if the regex has a capture group.
This reduces runtime of the check by almost 78 % for me.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
scripts/check-aclrules.py | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
...
# every func listed there, has an impl which calls
# an ACL function
if intable:
- assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''',
line)
+ assign = None
+ if '"' in line:
^This doesn't quite work, does it? I don't see any '"' in the driver
API
mapping structures. The only reliable pattern I see we could use is ', /* '
but in order to use that, we'd have to unify a few source files, mainly
vz_driver.c, doing:
git grep -Enpi ", \s+/\* " | grep driver.c
should help identifying them.
With this one addressed:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
12:37 p.m.
New subject: [libvirt] [PATCH 3/3] scripts: check-aclrules: use regular expressions less often
On Fri, Nov 22, 2019 at 09:00:12AM +0100, Erik Skultety wrote:
On Wed, Nov 20, 2019 at 07:32:46PM +0100, Ján Tomko wrote:
> Use a simple
> if "substr" in line
> before running a regular expression, which saves time,
> especially if the regex has a capture group.
>
> This reduces runtime of the check by almost 78 % for me.
>
> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> ---
> scripts/check-aclrules.py | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
...
> # every func listed there, has an impl which calls
> # an ACL function
> if intable:
> - assign =
re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line)
> + assign = None
> + if '"' in line:
^This doesn't quite work, does it? I don't see any '"' in the driver
API
mapping structures. The only reliable pattern I see we could use is ', /* '
but in order to use that, we'd have to unify a few source files, mainly
vz_driver.c, doing:
git grep -Enpi ", \s+/\* " | grep driver.c
should help identifying them.
Oops, that should've been "=".
I have no idea how I managed to type that equal sign sideways. =)
Jano
With this one addressed:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
1:15 p.m.
New subject: [libvirt] [PATCH 3/3] scripts: check-aclrules: use regular expressions less often
On Fri, Nov 22, 2019 at 11:37:31AM +0100, Ján Tomko wrote:
On Fri, Nov 22, 2019 at 09:00:12AM +0100, Erik Skultety wrote:
> On Wed, Nov 20, 2019 at 07:32:46PM +0100, Ján Tomko wrote:
> > Use a simple
> > if "substr" in line
> > before running a regular expression, which saves time,
> > especially if the regex has a capture group.
> >
> > This reduces runtime of the check by almost 78 % for me.
> >
> > Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> > ---
> > scripts/check-aclrules.py | 26 +++++++++++++++++++-------
> > 1 file changed, 19 insertions(+), 7 deletions(-)
> ...
>
> > # every func listed there, has an impl which calls
> > # an ACL function
> > if intable:
> > - assign =
re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line)
> > + assign = None
> > + if '"' in line:
>
> ^This doesn't quite work, does it? I don't see any '"' in the
driver API
> mapping structures. The only reliable pattern I see we could use is ', /* '
> but in order to use that, we'd have to unify a few source files, mainly
> vz_driver.c, doing:
>
> git grep -Enpi ", \s+/\* " | grep driver.c
>
> should help identifying them.
Oops, that should've been "=".
I have no idea how I managed to type that equal sign sideways. =)
I thought so, but with the pattern I suggested, you can narrow down the final
set of lines even more, so might turn out to be even faster than a plain "="
Erik
9:16 p.m.
New subject: [libvirt] [PATCH 4/3] scripts: use in even more
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
scripts/check-aclrules.py | 2 +-
scripts/check-driverimpls.py | 2 +-
scripts/check-symfile.py | 6 +++---
scripts/dtrace2systemtap.py | 4 ++--
scripts/genpolkit.py | 4 ++--
scripts/gensystemtap.py | 2 +-
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/scripts/check-aclrules.py b/scripts/check-aclrules.py
index 40c47c1c6b..1a76ebcb3b 100755
--- a/scripts/check-aclrules.py
+++ b/scripts/check-aclrules.py
@@ -128,7 +128,7 @@ with open(proto, "r") as fh:
api = name_to_ProcName(m.group(1))
# Event filtering is handled in daemon/remote.c
# instead of drivers
- if line.find("_EVENT_REGISTER") == -1:
+ if "_EVENT_REGISTER" not in line:
filteredmap[api] = True
incomment = False
diff --git a/scripts/check-driverimpls.py b/scripts/check-driverimpls.py
index ca7ec3af7f..bc3f16a816 100755
--- a/scripts/check-driverimpls.py
+++ b/scripts/check-driverimpls.py
@@ -33,7 +33,7 @@ def checkdriverimpls(filename):
for line in fh:
lineno = lineno + 1
if intable:
- if line.find("}") != -1:
+ if "}" in line:
intable = False
mainprefix = None
continue
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
index a543a0fbcd..0c02591991 100755
--- a/scripts/check-symfile.py
+++ b/scripts/check-symfile.py
@@ -36,9 +36,9 @@ ret = 0
with open(symfile, "r") as fh:
for line in fh:
line = line.strip()
- if line.find("{") != -1:
+ if "{" in line:
continue
- if line.find("}") != -1:
+ if "}" in line:
continue
if line in ["global:", "local:"]:
continue
@@ -46,7 +46,7 @@ with open(symfile, "r") as fh:
continue
if line[0] == '#':
continue
- if line.find("*") != -1:
+ if "*" in line:
continue
line = line.strip(";")
diff --git a/scripts/dtrace2systemtap.py b/scripts/dtrace2systemtap.py
index 922713e599..cf4d8e1a79 100755
--- a/scripts/dtrace2systemtap.py
+++ b/scripts/dtrace2systemtap.py
@@ -49,9 +49,9 @@ with open(dtrace, "r") as fh:
line = line.strip()
if line == "":
continue
- if line.find("provider ") != -1 and line.find("{") != -1:
+ if "provider " in line and "{" in line:
continue
- if line.find("};") != -1:
+ if "};" in line:
continue
if line.startswith("#"):
diff --git a/scripts/genpolkit.py b/scripts/genpolkit.py
index 0cdba2bd3c..230d920f70 100755
--- a/scripts/genpolkit.py
+++ b/scripts/genpolkit.py
@@ -55,13 +55,13 @@ aclfile = sys.argv[1]
with open(aclfile, "r") as fh:
for line in fh:
if in_opts:
- if line.find("*/") != -1:
+ if "*/" in line:
in_opts = False
else:
m = re.search(r'''\*\s*\@(\w+):\s*(.*?)\s*$''',
line)
if m is not None:
opts[m.group(1)] = m.group(2)
- elif line.find("**") != -1:
+ elif "**" in line:
in_opts = True
else:
m =
re.search(r'''VIR_ACCESS_PERM_(%s)_((?:\w|_)+),''' %
diff --git a/scripts/gensystemtap.py b/scripts/gensystemtap.py
index c4bc1620d0..7b391cc911 100755
--- a/scripts/gensystemtap.py
+++ b/scripts/gensystemtap.py
@@ -46,7 +46,7 @@ def load_file(fh):
instatus = True
elif re.search(r'''enum remote_auth_type''', line):
inauth = True
- elif line.find("}") != -1:
+ elif "}" in line:
intype = False
instatus = False
inauth = False
--
2.19.2
10:02 a.m.
New subject: [libvirt] [PATCH 4/3] scripts: use in even more
On Wed, Nov 20, 2019 at 08:16:27PM +0100, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
scripts/check-aclrules.py | 2 +-
scripts/check-driverimpls.py | 2 +-
scripts/check-symfile.py | 6 +++---
scripts/dtrace2systemtap.py | 4 ++--
scripts/genpolkit.py | 4 ++--
scripts/gensystemtap.py | 2 +-
6 files changed, 10 insertions(+), 10 deletions(-)
With the following squashed in:
diff --git a/scripts/dtrace2systemtap.py b/scripts/dtrace2systemtap.py
index cf4d8e1a79..d6bf1f8d1f 100755
--- a/scripts/dtrace2systemtap.py
+++ b/scripts/dtrace2systemtap.py
@@ -126,7 +126,7 @@ for file in filelist:
for idx in range(len(argbits)):
arg = argbits[idx]
isstr = False
- if arg.find("char *") != -1:
+ if 'char *' in arg:
isstr = True
m = re.search(r'''^.*\s\*?(\S+)$''', arg)
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
9:34 p.m.
On 11/20/19 3:32 PM, Ján Tomko wrote:
This shaves off 4 % of syntax-check runtime on my 8-core system,
which is enough of a reason for me.
Around 10% increase on my system running 'make -j syntax-check'.
All patches:
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
9:41 p.m.
On 11/20/19 4:34 PM, Daniel Henrique Barboza wrote:
On 11/20/19 3:32 PM, Ján Tomko wrote:
> This shaves off 4 % of syntax-check runtime on my 8-core system,
> which is enough of a reason for me.
Around 10% increase on my system running 'make -j syntax-check'.
In time: *performance* increase. Did a few runs, on average these patches
made syntax-check 10% faster for me (~11 seconds without, ~10 seconds with
them).
Thanks,
DHB
1895
days inactive
1897
days old
13 comments
3 participants
participants (3)
-
Daniel Henrique Barboza -
Erik Skultety -
Ján Tomko