From e380a4a8586da643bf4e0d2281b13aee0d5e5207 Mon Sep 17 00:00:00 2001 From: Franciszek Piszcz Date: Sun, 10 Jun 2018 16:35:49 +0200 Subject: [PATCH] Fix bottlenecks related to large number of files. --- dvc/binary_file_extensions.py | 51 +++++++++++++++++++++++++++++++++++ dvc/remote/local.py | 18 +++++++------ dvc/state.py | 13 ++++----- dvc/utils.py | 9 ++++++- 4 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 dvc/binary_file_extensions.py diff --git a/dvc/binary_file_extensions.py b/dvc/binary_file_extensions.py new file mode 100644 index 000000000..cecc8bbcf --- /dev/null +++ b/dvc/binary_file_extensions.py @@ -0,0 +1,51 @@ +# source: https://github.com/sindresorhus/binary-extensions +# +# MIT License +# +# Copyright (c) Sindre Sorhus (sindresorhus.com) +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +BINARY_FILE_EXTENSIONS = [ + "3ds", "3g2", "3gp", "7z", "a", "aac", "adp", "ai", "aif", "aiff", "alz", + "ape", "apk", "ar", "arj", "asf", "au", "avi", "bak", "baml", "bh", "bin", + "bk", "bmp", "btif", "bz2", "bzip2", "cab", "caf", "cgm", "class", "cmx", + "cpio", "cr2", "csv", "cur", "dat", "dcm", "deb", "dex", "djvu", "dll", + "dmg", "dng", "doc", "docm", "docx", "dot", "dotm", "dra", "DS_Store", + "dsk", "dts", "dtshd", "dvb", "dwg", "dxf", "ecelp4800", "ecelp7470", + "ecelp9600", "egg", "eol", "eot", "epub", "exe", "f4v", "fbs", "fh", "fla", + "flac", "fli", "flv", "fpx", "fst", "fvt", "g3", "gif", "graffle", "gz", + "gzip", "h261", "h263", "h264", "icns", "ico", "ief", "img", "ipa", "iso", + "jar", "jpeg", "jpg", "jpgv", "jpm", "jxr", "key", "ktx", "lha", "lib", + "lvp", "lz", "lzh", "lzma", "lzo", "m3u", "m4a", "m4v", "mar", "mdi", "mht", + "mid", "midi", "mj2", "mka", "mkv", "mmr", "mng", "mobi", "mov", "movie", + "mp3", "mp4", "mp4a", "mpeg", "mpg", "mpga", "mxu", "nef", "npx", "numbers", + "o", "oga", "ogg", "ogv", "otf", "pages", "pbm", "pcx", "pdb", "pdf", "pea", + "pgm", "pic", "png", "pnm", "pot", "potm", "potx", "ppa", "ppam", "ppm", + "pps", "ppsm", "ppsx", "ppt", "pptm", "pptx", "psd", "pya", "pyc", "pyo", + "pyv", "qt", "rar", "ras", "raw", "resources", "rgb", "rip", "rlc", "rmf", + "rmvb", "rtf", "rz", "s3m", "s7z", "scpt", "sgi", "shar", "sil", "sketch", + "slk", "smv", "so", "sub", "swf", "tar", "tbz", "tbz2", "tga", "tgz", + "thmx", "tif", "tiff", "tlz", "ttc", "ttf", "txz", "udf", "uvh", "uvi", + "uvm", "uvp", "uvs", "uvu", "viv", "vob", "war", "wav", "wax", "wbmp", + "wdp", "weba", "webm", "webp", "whl", "wim", "wm", "wma", "wmv", "wmx", + "woff", "woff2", "wvx", "xbm", "xif", "xla", "xlam", "xls", "xlsb", "xlsm", + "xlsx", "xlt", "xltm", "xltx", "xm", "xmind", "xpi", "xpm", "xwd", "xz", + "z", "zip", "zipx", +] diff --git a/dvc/remote/local.py b/dvc/remote/local.py index 2b6d17490..a30d625b1 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -68,7 +68,7 @@ class RemoteLOCAL(RemoteBase): return False - def link(self, src, link): + def link(self, src, link, dump=True): dname = os.path.dirname(link) if not os.path.exists(dname): os.makedirs(dname) @@ -83,12 +83,13 @@ class RemoteLOCAL(RemoteBase): msg = u'Checking out \'{}\' with cache \'{}\'' Logger.debug(msg.format(os.path.relpath(src), os.path.relpath(link))) self.CACHE_TYPE_MAP[typ](src, link) - self.link_state.update(link) + self.link_state.update(link, dump=dump) return except Exception as exc: msg = 'Cache type \'{}\' is not supported'.format(typ) Logger.debug(msg) if typ == types[-1]: + raise raise DvcException(msg, cause=exc) @staticmethod @@ -137,7 +138,7 @@ class RemoteLOCAL(RemoteBase): def is_dir_cache(cache): return cache.endswith(State.MD5_DIR_SUFFIX) - def checkout(self, path_info, checksum_info): + def checkout(self, path_info, checksum_info, dump=True): path = path_info['path'] md5 = checksum_info.get(self.PARAM_MD5, None) cache = self.get(md5) @@ -155,7 +156,7 @@ class RemoteLOCAL(RemoteBase): remove(path) if not self.is_dir_cache(cache): - self.link(cache, path) + self.link(cache, path, dump=dump) return msg = u'Checking out directory \'{}\' with cache \'{}\'' @@ -169,7 +170,7 @@ class RemoteLOCAL(RemoteBase): dir_cache = self.dir_cache(cache) for relpath, c in dir_cache.items(): p = os.path.join(path, relpath) - self.link(c, p) + self.link(c, p, dump=dump) def _move(self, inp, outp): # moving in two stages to make last the move atomic in @@ -178,7 +179,7 @@ class RemoteLOCAL(RemoteBase): move(inp, tmp) move(tmp, outp) - def _save_file(self, path_info): + def _save_file(self, path_info, dump=True): path = path_info['path'] md5 = self.state.update(path) cache = self.get(md5) @@ -187,7 +188,7 @@ class RemoteLOCAL(RemoteBase): os.path.relpath(cache))) self._move(path, cache) self.state.update(cache) - self.checkout(path_info, {self.PARAM_MD5: md5}) + self.checkout(path_info, {self.PARAM_MD5: md5}, dump=dump) return {self.PARAM_MD5: md5} @@ -202,7 +203,8 @@ class RemoteLOCAL(RemoteBase): relpath = entry[State.PARAM_RELPATH] p = os.path.join(path, relpath) - self._save_file({'scheme': 'local', 'path': p}) + self._save_file({'scheme': 'local', 'path': p}, dump=False) + self.link_state.dump() if not os.path.isdir(dname): os.makedirs(dname) diff --git a/dvc/state.py b/dvc/state.py index 0552fe0eb..43069f990 100644 --- a/dvc/state.py +++ b/dvc/state.py @@ -187,7 +187,7 @@ class LinkState(State): super(LinkState, self).__init__(dvc_dir) self.root_dir = root_dir - def update(self, path): + def update(self, path, dump=True): if not os.path.exists(path): return @@ -195,11 +195,12 @@ class LinkState(State): inode = self.inode(path) with self._lock: - with self._lock_file: - state = LinkStateEntry(inode, mtime) - d = state.dumpd() - self._db[os.path.relpath(path, self.root_dir)] = d - self.dump() + state = LinkStateEntry(inode, mtime) + d = state.dumpd() + self._db[os.path.relpath(path, self.root_dir)] = d + if dump: + with self._lock_file: + self.dump() def _do_remove_all(self): for p, s in self._db.items(): diff --git a/dvc/utils.py b/dvc/utils.py index debce3ec2..ea4ba37da 100644 --- a/dvc/utils.py +++ b/dvc/utils.py @@ -13,6 +13,7 @@ from multiprocessing.pool import ThreadPool from dvc.progress import progress from dvc.logger import Logger +from dvc.binary_file_extensions import BINARY_FILE_EXTENSIONS LOCAL_CHUNK_SIZE = 1024*1024 @@ -26,7 +27,13 @@ def file_md5(fname): """ get the (md5 hexdigest, md5 digest) of a file """ if os.path.exists(fname): hash_md5 = hashlib.md5() - binary = is_binary(fname) + # when dealing with large collections of binary files, the is_binary + # call becomes a bottleneck, here we first check the extension to + # avoid this bottleneck when possible: + if fname.split('.')[-1] in BINARY_FILE_EXTENSIONS: + binary = True + else: + binary = is_binary(fname) if binary: mode = "rb"