-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support PPE profiling for FileCache / MultiprocessFileCache #350
Conversation
aa6cffe
to
8a89949
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test codes are LGTM, but I have more suggestions for records.
pfio/cache/file_cache.py
Outdated
|
||
def _get(self, i): | ||
if i < 0 or self.length <= i: | ||
raise IndexError("index {} out of range ([0, {}])" | ||
.format(i, self.length - 1)) | ||
|
||
offset = self.buflen * i | ||
with self.lock.rdlock(): | ||
with record("pfio.cache.file:get:lock", trace=self.trace), \ | ||
self.lock.rdlock(): | ||
buf = os.pread(self.cachefp.fileno(), self.buflen, offset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better we add more records to track the actual I/O operation here because under get:lock
issues one rdlock and two pread operations.
pfio/cache/file_cache.py
Outdated
@@ -270,7 +276,8 @@ def _put(self, i, data): | |||
return False | |||
|
|||
offset = self.buflen * i | |||
with self.lock.wrlock(): | |||
with record("pfio.cache.file:put:lock", trace=self.trace), \ | |||
self.lock.wrlock(): | |||
buf = os.pread(self.cachefp.fileno(), self.buflen, offset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto as above
if l < 0 or o < 0: | ||
with record(f"pfio.cache.multiprocessfile:get:lock-{self.cache_fd}", | ||
trace=self.trace): | ||
index_entry = os.pread(self.cache_fd, self.buflen, offset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, we need to track this read call as well
warnings.warn(ose.strerror, RuntimeWarning) | ||
return False | ||
else: | ||
raise ose | ||
|
||
def _put(self, i, data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like file_cache, we'd like to track I/O-related and lock operations separately. Separating them will be useful for diagnosing whether I/O or just lock wait is long.
Signed-off-by: KantaTamura <[email protected]>
change the record range while acquiring lock Signed-off-by: KantaTamura <[email protected]>
Signed-off-by: KantaTamura <[email protected]>
Signed-off-by: KantaTamura <[email protected]>
Signed-off-by: KantaTamura <[email protected]>
0e47ace
to
7461588
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR resolves the
FileCache
/MultiprocessFileCache
task in #258Note
preload()
andpreserve()
are excluded from profiling because they are pre-processing and post-processing and have nothing to do with the I/O being executed.Sample program to check tracing
output
rendering of the output json file with chrome://tracing is shown below
trace_filecache.json
trace_multiprocess_filecache.json