Skip to content
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

Add block cache tracer. #5410

Closed
wants to merge 9 commits into from
Closed

Conversation

HaoyuHuang
Copy link
Contributor

This PR adds a help class block cache tracer to read/write block cache accesses. It uses the trace reader/writer to perform this task.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HaoyuHuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@HaoyuHuang has updated the pull request. Re-import the pull request

Copy link
Contributor

@riversand963 riversand963 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass.

trace_replay/block_cache_tracer.h Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.h Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.cc Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.cc Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.h Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@HaoyuHuang has updated the pull request. Re-import the pull request

@facebook-github-bot
Copy link
Contributor

@HaoyuHuang has updated the pull request. Re-import the pull request

Copy link
Contributor

@riversand963 riversand963 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another pass.
You may also need to update src.mk.

trace_replay/block_cache_tracer.h Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.cc Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.cc Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.h Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.h Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer_test.cc Show resolved Hide resolved
trace_replay/block_cache_tracer_test.cc Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@HaoyuHuang has updated the pull request. Re-import the pull request

@facebook-github-bot
Copy link
Contributor

@HaoyuHuang has updated the pull request. Re-import the pull request

@HaoyuHuang
Copy link
Contributor Author

I updated src.mk. I also added the code to perform the downsampling.

Copy link
Contributor

@riversand963 riversand963 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken another pass and left some comments.

src.mk Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer_test.cc Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer_test.cc Outdated Show resolved Hide resolved
src.mk Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.cc Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.cc Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.cc Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.cc Outdated Show resolved Hide resolved
trace_replay/block_cache_tracer.h Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@HaoyuHuang has updated the pull request. Re-import the pull request

@HaoyuHuang
Copy link
Contributor Author

Thanks for your comments.

Copy link
Contributor

@riversand963 riversand963 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a few minor, last comments.
Thanks @HaoyuHuang for the PR.

src.mk Outdated
@@ -371,6 +372,7 @@ MAIN_SOURCES = \
tools/reduce_levels_test.cc \
tools/sst_dump_test.cc \
tools/trace_analyzer_test.cc \
trace_replay/block_cache_tracer_test.cc \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may also want to adjust the alignment of \. It's not strictly required, but other lines are trying to align.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. I changed my editor to show indentation now. I updated it.

Status WriteHeader();

private:
bool ShouldTrace(const BlockCacheTraceRecord& record);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool ShouldTrace(const BlockCacheTraceRecord& record) const;?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Updated.

@facebook-github-bot
Copy link
Contributor

@HaoyuHuang has updated the pull request. Re-import the pull request

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HaoyuHuang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@HaoyuHuang merged this pull request in aa71718.

vagogte pushed a commit to vagogte/rocksdb that referenced this pull request Jun 18, 2019
Summary:
This PR adds a help class block cache tracer to read/write block cache accesses. It uses the trace reader/writer to perform this task.
Pull Request resolved: facebook#5410

Differential Revision: D15612843

Pulled By: HaoyuHuang

fbshipit-source-id: f30fd1e1524355ca87db5d533a5c086728b141ea
levichen94 pushed a commit to bytedance/terarkdb that referenced this pull request Jun 18, 2021
Summary:
This PR adds a help class block cache tracer to read/write block cache accesses. It uses the trace reader/writer to perform this task.
Pull Request resolved: facebook/rocksdb#5410

Differential Revision: D15612843

Pulled By: HaoyuHuang

fbshipit-source-id: f30fd1e1524355ca87db5d533a5c086728b141ea
Signed-off-by: Changlong Chen <levisonchen@live.cn>
mm304321141 pushed a commit to bytedance/terarkdb that referenced this pull request Jun 23, 2021
Summary:
This PR adds a help class block cache tracer to read/write block cache accesses. It uses the trace reader/writer to perform this task.
Pull Request resolved: facebook/rocksdb#5410

Differential Revision: D15612843

Pulled By: HaoyuHuang

fbshipit-source-id: f30fd1e1524355ca87db5d533a5c086728b141ea
Signed-off-by: Changlong Chen <levisonchen@live.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants