diff options
author | Tristan Van Berkom <tristan.vanberkom@codethink.co.uk> | 2018-09-10 16:14:51 +0900 |
---|---|---|
committer | Tristan Van Berkom <tristan.vanberkom@codethink.co.uk> | 2018-09-10 16:53:56 +0900 |
commit | cbfddc13ebe6a9be3cfe4bf62e9b996c45c5dcdc (patch) | |
tree | c76512420852f677463dbe1aa07371aa62226fa5 | |
parent | 791f7ddad656ce67022225a43c28927e3887b3a9 (diff) | |
download | buildstream-cbfddc13ebe6a9be3cfe4bf62e9b996c45c5dcdc.tar.gz |
element.py: Remove _get_artifact_size()
There is no justification to hold onto this state here.
Instead, just make `Element._assemble()` return the size of the
artifact it cached, and localize handling of that return value in
the BuildQueue implementation where the value is observed.
-rw-r--r-- | buildstream/_scheduler/jobs/elementjob.py | 3 | ||||
-rw-r--r-- | buildstream/_scheduler/queues/buildqueue.py | 31 | ||||
-rw-r--r-- | buildstream/element.py | 24 |
3 files changed, 22 insertions, 36 deletions
diff --git a/buildstream/_scheduler/jobs/elementjob.py b/buildstream/_scheduler/jobs/elementjob.py index 36527794e..b3318302a 100644 --- a/buildstream/_scheduler/jobs/elementjob.py +++ b/buildstream/_scheduler/jobs/elementjob.py @@ -109,14 +109,11 @@ class ElementJob(Job): data = {} workspace = self._element._get_workspace() - artifact_size = self._element._get_artifact_size() artifacts = self._element._get_artifact_cache() cache_size = artifacts.compute_cache_size() if workspace is not None: data['workspace'] = workspace.to_dict() - if artifact_size is not None: - data['artifact_size'] = artifact_size data['cache_size'] = cache_size return data diff --git a/buildstream/_scheduler/queues/buildqueue.py b/buildstream/_scheduler/queues/buildqueue.py index 6eb919082..8f593fea7 100644 --- a/buildstream/_scheduler/queues/buildqueue.py +++ b/buildstream/_scheduler/queues/buildqueue.py @@ -67,8 +67,7 @@ class BuildQueue(Queue): return super().enqueue(to_queue) def process(self, element): - element._assemble() - return element._get_unique_id() + return element._assemble() def status(self, element): # state of dependencies may have changed, recalculate element state @@ -87,18 +86,20 @@ class BuildQueue(Queue): return QueueStatus.READY - def _check_cache_size(self, job, element): - if not job.child_data: - return + def _check_cache_size(self, job, element, artifact_size): - artifact_size = job.child_data.get('artifact_size', False) + # After completing a build job, add the artifact size + # as returned from Element._assemble() to the estimated + # artifact cache size + # + cache = element._get_artifact_cache() + cache.add_artifact_size(artifact_size) - if artifact_size: - cache = element._get_artifact_cache() - cache.add_artifact_size(artifact_size) - - if cache.get_approximate_cache_size() > cache.cache_quota: - self._scheduler.check_cache_size() + # If the estimated size outgrows the quota, ask the scheduler + # to queue a job to actually check the real cache size. + # + if cache.get_approximate_cache_size() > cache.cache_quota: + self._scheduler.check_cache_size() def done(self, job, element, result, success): @@ -106,8 +107,8 @@ class BuildQueue(Queue): # Inform element in main process that assembly is done element._assemble_done() - # This has to be done after _assemble_done, such that the - # element may register its cache key as required - self._check_cache_size(job, element) + # This has to be done after _assemble_done, such that the + # element may register its cache key as required + self._check_cache_size(job, element, result) return True diff --git a/buildstream/element.py b/buildstream/element.py index 355220ff6..7ba680aac 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -213,7 +213,6 @@ class Element(Plugin): self.__staged_sources_directory = None # Location where Element.stage_sources() was called self.__tainted = None # Whether the artifact is tainted and should not be shared self.__required = False # Whether the artifact is required in the current session - self.__artifact_size = None # The size of data committed to the artifact cache self.__build_result = None # The result of assembling this Element self._build_log_path = None # The path of the build log for this Element @@ -1509,6 +1508,9 @@ class Element(Plugin): # - Call the public abstract methods for the build phase # - Cache the resulting artifact # + # Returns: + # (int): The size of the newly cached artifact + # def _assemble(self): # Assert call ordering @@ -1655,7 +1657,7 @@ class Element(Plugin): }), os.path.join(metadir, 'workspaced-dependencies.yaml')) with self.timed_activity("Caching artifact"): - self.__artifact_size = utils._get_dir_size(assembledir) + artifact_size = utils._get_dir_size(assembledir) self.__artifacts.commit(self, assembledir, self.__get_cache_keys_for_commit()) if collect is not None and collectvdir is None: @@ -1667,6 +1669,8 @@ class Element(Plugin): # Finally cleanup the build dir cleanup_rootdir() + return artifact_size + def _get_build_log(self): return self._build_log_path @@ -1908,22 +1912,6 @@ class Element(Plugin): workspaces = self._get_context().get_workspaces() return workspaces.get_workspace(self._get_full_name()) - # _get_artifact_size() - # - # Get the size of the artifact produced by this element in the - # current pipeline - if this element has not been assembled or - # pulled, this will be None. - # - # Note that this is the size of an artifact *before* committing it - # to the cache, the size on disk may differ. It can act as an - # approximate guide for when to do a proper size calculation. - # - # Returns: - # (int|None): The size of the artifact - # - def _get_artifact_size(self): - return self.__artifact_size - # _get_artifact_cache() # # Accessor for the artifact cache |