> On March 20, 2018, 9:20 p.m., Reza Motamedi wrote: > > src/main/python/apache/aurora/executor/common/executor_detector.py > > Lines 28-85 (original), 26-38 (patched) > > <https://reviews.apache.org/r/66139/diff/1/?file=1982446#file1982446line29> > > > > We are using the functionalities here. Is the performance tunning > > related to this code delete?
Oh interesting. Do you use all removed functionality? The performance improvement is only partially related. There are cycles wasted in the translation from `path` to the parsing result of `ScanfParser` and back again, but the dominant factor is the removed `realpath` in `path_detector.py`. I should be able to rewrite the patch so that `executor_detector.py` remains unchanged. > On March 20, 2018, 9:20 p.m., Reza Motamedi wrote: > > src/main/python/apache/aurora/executor/common/path_detector.py > > Lines 31-33 (original), 31-33 (patched) > > <https://reviews.apache.org/r/66139/diff/1/?file=1982447#file1982447line31> > > > > Curious if this generator pattern is really useful. Don't we just > > consume everything it generates? We need the result of `os.path.join(path, self._sandbox_path)` as a variable, in order to both return it and check via `os.path.exists(path)`. An alternative would be to use an explicit for loop here. - Stephan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66139/#review199564 ----------------------------------------------------------- On March 20, 2018, 12:22 a.m., Stephan Erb wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66139/ > ----------------------------------------------------------- > > (Updated March 20, 2018, 12:22 a.m.) > > > Review request for Aurora, Jordan Ly, Renan DelValle, and Reza Motamedi. > > > Repository: aurora > > > Description > ------- > > Profiling indicated that roughly 70% of the refresh time was spend in > `os.path.realpath`. > This was introduced in https://reviews.apache.org/r/35580/ to properly handle > the `latest` > symlink in the Mesos folder layout. > > This patch takes a slightly different approach to solve this problem based on > `os.path.islink`. > The latter is faster as it just needs to look at a single folder rather than > an entire path. > > To facilitate this optimization the patch removes unused functionality from > ExecutorDetector. > Most probably this was used by former GC executor that got removed a few > years ago. > > > Diffs > ----- > > src/main/python/apache/aurora/executor/BUILD > 486230db34a22ea5dd0f68da911c0afb1afbcac0 > src/main/python/apache/aurora/executor/common/executor_detector.py > a07bfc34caa5f86153ace8184b061e253c39e92e > src/main/python/apache/aurora/executor/common/path_detector.py > ed264d74ef5a5a7aa681a56b340f9b16504a88ad > src/main/python/apache/thermos/monitoring/detector.py > 6e5a620e9f49e0960a814f4c1f701a21cc7678fd > src/test/python/apache/aurora/executor/common/test_executor_detector.py > d2a948f2e95cbc1723b63462787c4256cc56731d > src/test/python/apache/aurora/executor/common/test_path_detector.py > 7b5ef0cf552d22d4cfbf3357071de036551026dc > > > Diff: https://reviews.apache.org/r/66139/diff/1/ > > > Testing > ------- > > I have tested this build on a node with 75 running tasks and 4500 finished > ones. > > Before this patch: > > D0319 09:05:07.219057 14514 task_observer.py:142] TaskObserver: finished > checkpoint refresh in 2.26s > D0319 09:05:14.412610 14514 task_observer.py:142] TaskObserver: finished > checkpoint refresh in 2.39s > D0319 09:05:21.600985 14514 task_observer.py:142] TaskObserver: finished > checkpoint refresh in 2.29s > > With this patch: > > D0319 09:00:24.084657 5427 task_observer.py:142] TaskObserver: finished > checkpoint refresh in 0.77s > D0319 09:00:29.869699 5427 task_observer.py:142] TaskObserver: finished > checkpoint refresh in 0.78s > D0319 09:00:35.643407 5427 task_observer.py:142] TaskObserver: finished > checkpoint refresh in 0.77s > > The regular 2-3 second freezes when navigating the Thermos UI are now almost > gone for me. > > > Thanks, > > Stephan Erb > >
