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

Best offset prefetcher #10

Open
wants to merge 6 commits into
base: memtrace
Choose a base branch
from

Conversation

maxwellrbradley
Copy link

This branch contains the implementation of the best offset prefetcher.

Copy link

@hlitz hlitz left a comment

Choose a reason for hiding this comment

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

mostly nits. The only feature request I have is some stats and potentially some useful configuration parameters.

src/best_offset_prefetcher.cpp Outdated Show resolved Hide resolved
src/best_offset_prefetcher.cpp Outdated Show resolved Hide resolved
src/best_offset_prefetcher.cpp Outdated Show resolved Hide resolved
src/best_offset_prefetcher.cpp Outdated Show resolved Hide resolved
src/best_offset_prefetcher.cpp Outdated Show resolved Hide resolved
src/best_offset_prefetcher.cpp Outdated Show resolved Hide resolved

// num run throughs to make before round over
const uint64_t RND_MAX = 100;
//const uint64_t RND_MAX = 50;
Copy link

Choose a reason for hiding this comment

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

remove multiple constants. If you believe these should be configurable, add a new parameter that can be defined via the cfg file.

@@ -76,7 +76,7 @@ void TraceReaderMemtrace::binaryGroupPathIs(const std::string &_path) {
error.c_str());
return;
}
module_mapper_ = module_mapper_t::create(directory_.modfile_bytes,
Copy link

Choose a reason for hiding this comment

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

hmm is this a current bug? If yes please extract it into a separate commit/pull request

src/best_offset_prefetcher.cpp Outdated Show resolved Hide resolved
uint64_t current_round;
uint64_t test_offset_index;
uint64_t current_offset;
#ifndef TEST
Copy link

Choose a reason for hiding this comment

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

remove and add some actual stats (look how the other prefetchers are doing it).

Can you think of other useful stats besides emitted prefetches? This is very helpful for quick debugging the out file if things do not perform like expected.

Copy link

@hlitz hlitz left a comment

Choose a reason for hiding this comment

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

Please merge with the prior commit, and do another pull request consisting of a single commit.

…ved is to add configurability to the prefetcher from the configuration files.
Cleaned up code a bit.

Cleand up syntax of source files. Next step before pull request approved is to add configurability to the prefetcher from the configuration files.

Fixed up code to standards.

Added best offset prefetcher source files to forked branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants