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
Benchmark custom #2345
base: master
Are you sure you want to change the base?
Benchmark custom #2345
Changes from 3 commits
1a771bb
1b653aa
3e5bffd
517bdea
84566c1
0c43c86
7d67961
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,8 @@ private static final class DateFormatInfo { | |
private ArrayList<Path> inputFiles = new ArrayList<>(); | ||
private int nextFile = 0; | ||
private int iteration = 0; | ||
private int[] threadIndex; | ||
private volatile boolean threadIndexCreated; | ||
|
||
@Override | ||
public void setConfig(Config config) { | ||
|
@@ -102,19 +104,43 @@ public void close() throws IOException { | |
public DocData getNextDocData(DocData docData) throws NoMoreDataException, IOException { | ||
Path f = null; | ||
String name = null; | ||
synchronized (this) { | ||
if (nextFile >= inputFiles.size()) { | ||
// exhausted files, start a new round, unless forever set to false. | ||
if (!forever) { | ||
throw new NoMoreDataException(); | ||
} | ||
nextFile = 0; | ||
iteration++; | ||
} | ||
f = inputFiles.get(nextFile++); | ||
name = f.toRealPath() + "_" + iteration; | ||
int inputFilesSize = inputFiles.size(); | ||
|
||
/* | ||
* synchronized (this) { | ||
* if (nextFile >= inputFiles.size()) { // exhausted files, start a new round, unless forever set to false. | ||
* if (!forever) { | ||
* throw new NoMoreDataException(); | ||
* } | ||
* nextFile = 0; | ||
* iteration++; | ||
* } | ||
* f = inputFiles.get(nextFile++); | ||
* name = f.toRealPath() + "_" +iteration; | ||
* } | ||
*/ | ||
if (!threadIndexCreated) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, will do the required changes. |
||
createThreadIndex(); | ||
} | ||
|
||
int index = (int) Thread.currentThread().getId() % threadIndex.length; | ||
int fIndex = index + threadIndex[index] * threadIndex.length; | ||
threadIndex[index]++; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused how this approach ensures that we will indeed index every document in the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although, getId() is controlled by JVM but in our case, all threadIndex are getting initialized at once. Hence, there is high chance of getting guaranteed sequence of thread id, as we also observed. However, we understand your concern and tweaked our code in such a way that it guaranteed to reach every possible int from 0 .. threadIndex.length. We achieved it by setting a unique thread name and parsing the same for calculating the index value. |
||
|
||
// Sanity check, if # threads is greater than # input files, wrap index | ||
if (index >= inputFilesSize) index %= inputFilesSize; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you move the |
||
|
||
// Check if this thread has exhausted its files | ||
if (fIndex >= inputFilesSize) { | ||
threadIndex[index] = 0; | ||
fIndex = index + threadIndex[index] * threadIndex.length; | ||
threadIndex[index]++; | ||
iteration++; | ||
} | ||
|
||
f = inputFiles.get(fIndex); | ||
name = f.toRealPath() + "_" + iteration; | ||
|
||
try (BufferedReader reader = Files.newBufferedReader(f, StandardCharsets.UTF_8)) { | ||
// First line is the date, 3rd is the title, rest is body | ||
String dateStr = reader.readLine(); | ||
|
@@ -146,4 +172,11 @@ public synchronized void resetInputs() throws IOException { | |
nextFile = 0; | ||
iteration = 0; | ||
} | ||
|
||
private synchronized void createThreadIndex() { | ||
if (!threadIndexCreated) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, will do the required changes. |
||
threadIndex = new int[getConfig().getNumThreads()]; | ||
threadIndexCreated = true; | ||
} | ||
} | ||
} |
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.
Just delete this old code? You are replacing it with a more concurrent version, yay!
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.
Sure, will delete the commented codes.