From 0749112286bb28a22f8c455df8be2e4821ab5d08 Mon Sep 17 00:00:00 2001 From: Cristian Ciupitu Date: Thu, 23 Jan 2014 17:02:17 +0200 Subject: [PATCH] Micro-optimization: replace s.find(x)!=-1 with x in s timeit shows a speedup of ~3x on Python 2.7.5 x86_64. It also makes the code a bit shorter. --- bin/ansible-galaxy | 2 +- docsite/rst/developing_modules.rst | 2 +- hacking/module_formatter.py | 2 +- lib/ansible/callbacks.py | 2 +- lib/ansible/inventory/ini.py | 16 ++++++++-------- lib/ansible/module_common.py | 4 ++-- lib/ansible/playbook/play.py | 2 +- lib/ansible/runner/__init__.py | 8 ++++---- lib/ansible/runner/action_plugins/async.py | 2 +- lib/ansible/runner/action_plugins/copy.py | 2 +- lib/ansible/runner/action_plugins/script.py | 2 +- lib/ansible/runner/connection_plugins/ssh.py | 9 +++++---- lib/ansible/utils/__init__.py | 17 +++++++++-------- library/cloud/virt | 2 +- library/commands/command | 2 +- library/database/mysql_user | 2 +- library/files/file | 2 +- library/system/open_iscsi | 2 +- library/system/service | 18 +++++++++--------- 19 files changed, 50 insertions(+), 48 deletions(-) diff --git a/bin/ansible-galaxy b/bin/ansible-galaxy index a528b950f83..7b346ac6e44 100755 --- a/bin/ansible-galaxy +++ b/bin/ansible-galaxy @@ -655,7 +655,7 @@ def execute_install(args, options, parser): if role_name == "" or role_name.startswith("#"): continue - elif role_name.find(',') != -1: + elif ',' in role_name: role_name,role_version = role_name.split(',',1) role_name = role_name.strip() role_version = role_version.strip() diff --git a/docsite/rst/developing_modules.rst b/docsite/rst/developing_modules.rst index 3f1c1e68dca..e8da717aed5 100644 --- a/docsite/rst/developing_modules.rst +++ b/docsite/rst/developing_modules.rst @@ -123,7 +123,7 @@ a lot shorter than this:: for arg in arguments: # ignore any arguments without an equals in it - if arg.find("=") != -1: + if "=" in arg: (key, value) = arg.split("=") diff --git a/hacking/module_formatter.py b/hacking/module_formatter.py index d5ed3031508..0a36c3951ca 100755 --- a/hacking/module_formatter.py +++ b/hacking/module_formatter.py @@ -185,7 +185,7 @@ def process_module(module, options, env, template, outputname, module_map): fname = module_map[module] # ignore files with extensions - if os.path.basename(fname).find(".") != -1: + if "." in os.path.basename(fname): return # use ansible core library to parse out doc metadata YAML and plaintext examples diff --git a/lib/ansible/callbacks.py b/lib/ansible/callbacks.py index b56d1f90695..c1b41a2a92f 100644 --- a/lib/ansible/callbacks.py +++ b/lib/ansible/callbacks.py @@ -250,7 +250,7 @@ def regular_generic_msg(hostname, result, oneline, caption): def banner_cowsay(msg): - if msg.find(": [") != -1: + if ": [" in msg: msg = msg.replace("[","") if msg.endswith("]"): msg = msg[:-1] diff --git a/lib/ansible/inventory/ini.py b/lib/ansible/inventory/ini.py index 718fee1338d..3b38911d253 100644 --- a/lib/ansible/inventory/ini.py +++ b/lib/ansible/inventory/ini.py @@ -69,7 +69,7 @@ class InventoryParser(object): line = utils.before_comment(line).strip() if line.startswith("[") and line.endswith("]"): active_group_name = line.replace("[","").replace("]","") - if line.find(":vars") != -1 or line.find(":children") != -1: + if ":vars" in line or ":children" in line: active_group_name = active_group_name.rsplit(":", 1)[0] if active_group_name not in self.groups: new_group = self.groups[active_group_name] = Group(name=active_group_name) @@ -95,11 +95,11 @@ class InventoryParser(object): # FQDN foo.example.com if hostname.count(".") == 1: (hostname, port) = hostname.rsplit(".", 1) - elif (hostname.find("[") != -1 and - hostname.find("]") != -1 and - hostname.find(":") != -1 and + elif ("[" in hostname and + "]" in hostname and + ":" in hostname and (hostname.rindex("]") < hostname.rindex(":")) or - (hostname.find("]") == -1 and hostname.find(":") != -1)): + ("]" not in hostname and ":" in hostname)): (hostname, port) = hostname.rsplit(":", 1) hostnames = [] @@ -152,7 +152,7 @@ class InventoryParser(object): line = line.strip() if line is None or line == '': continue - if line.startswith("[") and line.find(":children]") != -1: + if line.startswith("[") and ":children]" in line: line = line.replace("[","").replace(":children]","") group = self.groups.get(line, None) if group is None: @@ -177,7 +177,7 @@ class InventoryParser(object): group = None for line in self.lines: line = line.strip() - if line.startswith("[") and line.find(":vars]") != -1: + if line.startswith("[") and ":vars]" in line: line = line.replace("[","").replace(":vars]","") group = self.groups.get(line, None) if group is None: @@ -189,7 +189,7 @@ class InventoryParser(object): elif line == '': pass elif group: - if line.find("=") == -1: + if "=" not in line: raise errors.AnsibleError("variables assigned to group must be in key=value form") else: (k, v) = [e.strip() for e in line.split("=", 1)] diff --git a/lib/ansible/module_common.py b/lib/ansible/module_common.py index da02882d935..a6af86d6fcb 100644 --- a/lib/ansible/module_common.py +++ b/lib/ansible/module_common.py @@ -95,7 +95,7 @@ class ModuleReplacer(object): for line in lines: - if line.find(REPLACER) != -1: + if REPLACER in line: output.write(self.slurp(os.path.join(self.snippet_path, "basic.py"))) snippet_names.append('basic') elif line.startswith('from ansible.module_utils.'): @@ -103,7 +103,7 @@ class ModuleReplacer(object): import_error = False if len(tokens) != 3: import_error = True - if line.find(" import *") == -1: + if " import *" not in line: import_error = True if import_error: raise errors.AnsibleError("error importing module in %s, expecting format like 'from ansible.module_utils.basic import *'" % module_path) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 2289b0a4d3c..c1f5ba4e86f 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -707,7 +707,7 @@ class Play(object): # ************************************************* def _has_vars_in(self, msg): - return ((msg.find("$") != -1) or (msg.find("{{") != -1)) + return "$" in msg or "{{" in msg # ************************************************* diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index 27314ed8cad..385cdd141c1 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -415,7 +415,7 @@ class Runner(object): environment_string = self._compute_environment_string(inject) - if tmp.find("tmp") != -1 and ((self.sudo and self.sudo_user != 'root') or (self.su and self.su_user != 'root')): + if "tmp" in tmp and ((self.sudo and self.sudo_user != 'root') or (self.su and self.su_user != 'root')): # deal with possible umask issues once sudo'ed to other user cmd_chmod = "chmod a+r %s" % remote_module_path self._low_level_exec_command(conn, cmd_chmod, tmp, sudoable=False) @@ -469,7 +469,7 @@ class Runner(object): cmd = " ".join([environment_string.strip(), shebang.replace("#!","").strip(), cmd]) cmd = cmd.strip() - if tmp.find("tmp") != -1 and not C.DEFAULT_KEEP_REMOTE_FILES and not persist_files and delete_remote_tmp: + if "tmp" in tmp and not C.DEFAULT_KEEP_REMOTE_FILES and not persist_files and delete_remote_tmp: if not self.sudo or self.su or self.sudo_user == 'root' or self.su_user == 'root': # not sudoing or sudoing to root, so can cleanup files in the same step cmd = cmd + "; rm -rf %s >/dev/null 2>&1" % tmp @@ -485,7 +485,7 @@ class Runner(object): else: res = self._low_level_exec_command(conn, cmd, tmp, sudoable=sudoable, in_data=in_data) - if tmp.find("tmp") != -1 and not C.DEFAULT_KEEP_REMOTE_FILES and not persist_files and delete_remote_tmp: + if "tmp" in tmp and not C.DEFAULT_KEEP_REMOTE_FILES and not persist_files and delete_remote_tmp: if (self.sudo and self.sudo_user != 'root') or (self.su and self.su_user != 'root'): # not sudoing to root, so maybe can't delete files as that other user # have to clean up temp files as original user in a second step @@ -883,7 +883,7 @@ class Runner(object): return False def _late_needs_tmp_path(self, conn, tmp, module_style): - if tmp.find("tmp") != -1: + if "tmp" in tmp: # tmp has already been created return False if not conn.has_pipelining or not C.ANSIBLE_SSH_PIPELINING or C.DEFAULT_KEEP_REMOTE_FILES or self.su: diff --git a/lib/ansible/runner/action_plugins/async.py b/lib/ansible/runner/action_plugins/async.py index 12fe279a471..ac0d6e84928 100644 --- a/lib/ansible/runner/action_plugins/async.py +++ b/lib/ansible/runner/action_plugins/async.py @@ -33,7 +33,7 @@ class ActionModule(object): module_name = 'command' module_args += " #USE_SHELL" - if tmp.find("tmp") == -1: + if "tmp" not in tmp: tmp = self.runner._make_tmp_path(conn) (module_path, is_new_style, shebang) = self.runner._copy_module(conn, tmp, module_name, module_args, inject, complex_args=complex_args) diff --git a/lib/ansible/runner/action_plugins/copy.py b/lib/ansible/runner/action_plugins/copy.py index 79acdaba587..d395d1df6f5 100644 --- a/lib/ansible/runner/action_plugins/copy.py +++ b/lib/ansible/runner/action_plugins/copy.py @@ -331,7 +331,7 @@ class ActionModule(object): src = open(source) src_contents = src.read(8192) st = os.stat(source) - if src_contents.find("\x00") != -1: + if "\x00" in src_contents: diff['src_binary'] = 1 elif st[stat.ST_SIZE] > utils.MAX_FILE_SIZE_FOR_DIFF: diff['src_larger'] = utils.MAX_FILE_SIZE_FOR_DIFF diff --git a/lib/ansible/runner/action_plugins/script.py b/lib/ansible/runner/action_plugins/script.py index 149be3cc113..f50e2b08d6f 100644 --- a/lib/ansible/runner/action_plugins/script.py +++ b/lib/ansible/runner/action_plugins/script.py @@ -128,7 +128,7 @@ class ActionModule(object): result = handler.run(conn, tmp, 'raw', module_args, inject) # clean up after - if tmp.find("tmp") != -1 and not C.DEFAULT_KEEP_REMOTE_FILES: + if "tmp" in tmp and not C.DEFAULT_KEEP_REMOTE_FILES: self.runner._low_level_exec_command(conn, 'rm -rf %s >/dev/null 2>&1' % tmp, tmp) result.result['changed'] = True diff --git a/lib/ansible/runner/connection_plugins/ssh.py b/lib/ansible/runner/connection_plugins/ssh.py index 22189caadf3..cc548a1c9b2 100644 --- a/lib/ansible/runner/connection_plugins/ssh.py +++ b/lib/ansible/runner/connection_plugins/ssh.py @@ -68,9 +68,9 @@ class Connection(object): cp_in_use = False cp_path_set = False for arg in self.common_args: - if arg.find("ControlPersist") != -1: + if "ControlPersist" in arg: cp_in_use = True - if arg.find("ControlPath") != -1: + if "ControlPath" in arg: cp_path_set = True if cp_in_use and not cp_path_set: @@ -137,7 +137,7 @@ class Connection(object): data = host_fh.read() host_fh.close() for line in data.split("\n"): - if line is None or line.find(" ") == -1: + if line is None or " " not in line: continue tokens = line.split() if tokens[0].find(self.HASHED_KEY_MAGIC) == 0: @@ -324,7 +324,8 @@ class Connection(object): # the host to known hosts is not intermingled with multiprocess output. fcntl.lockf(self.runner.output_lockfile, fcntl.LOCK_UN) fcntl.lockf(self.runner.process_lockfile, fcntl.LOCK_UN) - controlpersisterror = stderr.find('Bad configuration option: ControlPersist') != -1 or stderr.find('unknown configuration option: ControlPersist') != -1 + controlpersisterror = 'Bad configuration option: ControlPersist' in stderr or \ + 'unknown configuration option: ControlPersist' in stderr if C.HOST_KEY_CHECKING: if ssh_cmd[0] == "sshpass" and p.returncode == 6: diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index 6c2f8112aba..476622e6766 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -193,7 +193,7 @@ def check_conditional(conditional, basedir, inject, fail_on_undefined=False): conditional = conditional.replace("jinja2_compare ","") # allow variable names - if conditional in inject and str(inject[conditional]).find('-') == -1: + if conditional in inject and '-' not in str(inject[conditional]): conditional = inject[conditional] conditional = template.template(basedir, conditional, inject, fail_on_undefined=fail_on_undefined) original = str(conditional).replace("jinja2_compare ","") @@ -206,9 +206,9 @@ def check_conditional(conditional, basedir, inject, fail_on_undefined=False): # variable was undefined. If we happened to be # looking for an undefined variable, return True, # otherwise fail - if conditional.find("is undefined") != -1: + if "is undefined" in conditional: return True - elif conditional.find("is defined") != -1: + elif "is defined" in conditional: return False else: raise errors.AnsibleError("error while evaluating conditional: %s" % original) @@ -331,9 +331,9 @@ def parse_json(raw_data): def smush_braces(data): ''' smush Jinaj2 braces so unresolved templates like {{ foo }} don't get parsed weird by key=value code ''' - while data.find('{{ ') != -1: + while '{{ ' in data: data = data.replace('{{ ', '{{') - while data.find(' }}') != -1: + while ' }}' in data: data = data.replace(' }}', '}}') return data @@ -374,7 +374,7 @@ def parse_yaml(data, path_hint=None): def process_common_errors(msg, probline, column): replaced = probline.replace(" ","") - if replaced.find(":{{") != -1 and replaced.find("}}") != -1: + if ":{{" in replaced and "}}" in replaced: msg = msg + """ This one looks easy to fix. YAML thought it was looking for the start of a hash/dictionary and was confused to see a second "{". Most likely this was @@ -542,7 +542,7 @@ def parse_kv(args): vargs = [x.decode('utf-8') for x in shlex.split(args, posix=True)] #vargs = shlex.split(str(args), posix=True) for x in vargs: - if x.find("=") != -1: + if "=" in x: k, v = x.split("=",1) options[k]=v return options @@ -1023,7 +1023,7 @@ def listify_lookup_plugin_terms(terms, basedir, inject): # not sure why the "/" is in above code :) try: new_terms = template.template(basedir, "{{ %s }}" % terms, inject) - if isinstance(new_terms, basestring) and new_terms.find("{{") != -1: + if isinstance(new_terms, basestring) and "{{" in new_terms.find: pass else: terms = new_terms @@ -1097,3 +1097,4 @@ def before_comment(msg): return msg + diff --git a/library/cloud/virt b/library/cloud/virt index 3400c3ff727..78d2aa1ab91 100644 --- a/library/cloud/virt +++ b/library/cloud/virt @@ -120,7 +120,7 @@ class LibvirtConnection(object): cmd = "uname -r" rc, stdout, stderr = self.module.run_command(cmd) - if stdout.find("xen") != -1: + if "xen" in stdout: conn = libvirt.open(None) else: conn = libvirt.open(uri) diff --git a/library/commands/command b/library/commands/command index ba9ae30a7f2..b35501f1bf8 100644 --- a/library/commands/command +++ b/library/commands/command @@ -180,7 +180,7 @@ class CommandModule(AnsibleModule): params['removes'] = None params['shell'] = False params['executable'] = None - if args.find("#USE_SHELL") != -1: + if "#USE_SHELL" in args: args = args.replace("#USE_SHELL", "") params['shell'] = True diff --git a/library/database/mysql_user b/library/database/mysql_user index e7fad3d77c6..b7c84fd1c3e 100644 --- a/library/database/mysql_user +++ b/library/database/mysql_user @@ -259,7 +259,7 @@ def privileges_unpack(priv): output = {} for item in priv.split('/'): pieces = item.split(':') - if pieces[0].find('.') != -1: + if '.' in pieces[0]: pieces[0] = pieces[0].split('.') for idx, piece in enumerate(pieces): if pieces[0][idx] != "*": diff --git a/library/files/file b/library/files/file index 7a038c9f362..65a3f417c03 100644 --- a/library/files/file +++ b/library/files/file @@ -168,7 +168,7 @@ def main(): f = open(path) b = f.read(8192) f.close() - if b.find("\x00") != -1: + if "\x00" in b: appears_binary = True except: pass diff --git a/library/system/open_iscsi b/library/system/open_iscsi index 2e57727cf59..3fd2b1a5a21 100644 --- a/library/system/open_iscsi +++ b/library/system/open_iscsi @@ -138,7 +138,7 @@ def iscsi_get_cached_nodes(module, portal=None): # older versions of scsiadm don't have nice return codes # for newer versions see iscsiadm(8); also usr/iscsiadm.c for details # err can contain [N|n]o records... - elif rc == 21 or (rc == 255 and err.find("o records found") != -1): + elif rc == 21 or (rc == 255 and "o records found" in err): nodes = [] else: module.fail_json(cmd=cmd, rc=rc, msg=err) diff --git a/library/system/service b/library/system/service index 5180a14d82b..ed30b72aa5b 100644 --- a/library/system/service +++ b/library/system/service @@ -483,9 +483,9 @@ class LinuxService(Service): if self.svc_initctl and self.running is None: # check the job status by upstart response initctl_rc, initctl_status_stdout, initctl_status_stderr = self.execute_command("%s status %s" % (self.svc_initctl, self.name)) - if initctl_status_stdout.find("stop/waiting") != -1: + if "stop/waiting" in initctl_status_stdout: self.running = False - elif initctl_status_stdout.find("start/running") != -1: + elif "start/running" in initctl_status_stdout: self.running = True if self.svc_cmd and self.svc_cmd.endswith("rc-service") and self.running is None: @@ -525,7 +525,7 @@ class LinuxService(Service): # if the job status is still not known check it by special conditions if self.running is None: - if self.name == 'iptables' and status_stdout.find("ACCEPT") != -1: + if self.name == 'iptables' and "ACCEPT" in status_stdout: # iptables status command output is lame # TODO: lookup if we can use a return code for this instead? self.running = True @@ -631,16 +631,16 @@ class LinuxService(Service): if line.startswith('rename'): self.changed = True break - elif self.enable and line.find('do not exist') != -1: + elif self.enable and 'do not exist' in line: self.changed = True break - elif not self.enable and line.find('already exist') != -1: + elif not self.enable and 'already exist' in line: self.changed = True break # Debian compatibility for line in err.splitlines(): - if self.enable and line.find('no runlevel symlinks to modify') != -1: + if self.enable and 'no runlevel symlinks to modify' in line: self.changed = True break @@ -982,9 +982,9 @@ class SunOSService(Service): # enabled false for line in stdout.split("\n"): if line.find("enabled") == 0: - if line.find("true") != -1: + if "true" in line: enabled = True - if line.find("temporary") != -1: + if "temporary" in line: temporary = True startup_enabled = (enabled and not temporary) or (not enabled and temporary) @@ -1176,7 +1176,7 @@ def main(): (rc, out, err) = service.modify_service_state() if rc != 0: - if err and err.find("is already") != -1: + if err and "is already" in err: # upstart got confused, one such possibility is MySQL on Ubuntu 12.04 # where status may report it has no start/stop links and we could # not get accurate status