-
Notifications
You must be signed in to change notification settings - Fork 180
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
Dev/ashish #753
Dev/ashish #753
Conversation
Old schedule/data choice changed to schedule/data SearchUnit Added new class for schedule/data Choice, which is just a wrapper around PMachineId/PValue<?> TODO: clean up Schedule with new class structure
This reverts commit 53c5d15.
[PEx] Make search strategies subclasses thread safe, minor refactoring
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.
Please undo changes in this file. It is outside the scope of PExplicit-related code.
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.
Let's remove this file as well.
Now that there is a function |
* Mapping from machine type to list of all machine instances | ||
*/ | ||
@Getter | ||
private final Map<Integer, Map<Class<? extends PMachine>, List<PMachine>>> machineListByTypePerThread = new ConcurrentHashMap<>(); // This is per thread; so make this map of tiD to same Map |
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.
Why is this per thread? Isn't each Scheduler object already per thread?
* Set of machines | ||
*/ | ||
@Getter | ||
private final Map<Integer, SortedSet<PMachine>> machineSetPerThread = new ConcurrentHashMap<>(); |
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.
Same. Why is this per thread? Isn't each Scheduler object already per thread?
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.
We should directly use PExplicit.getScheduler()
throughout this and other files, instead of first getting localTID with PExplicit.getTId_toLocalTID()
and then getting the Scheduler object using PExplicit.getSchedulers().get(localTID)
.
public BugFoundException(String message) { | ||
super(message); | ||
buggyLocalTID = (PExplicitGlobal.getTID_to_localtID()).get(Thread.currentThread().getId()); |
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.
Same. Directly use PExplicitGlobal.getScheduler()
.
PExplicitLogger.logBugFound(message); | ||
} | ||
|
||
public BugFoundException(String message, Throwable cause) { | ||
super(message, cause); | ||
buggyLocalTID = (PExplicitGlobal.getTID_to_localtID()).get(Thread.currentThread().getId()); |
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.
Same. Directly use PExplicitGlobal.getScheduler()
.
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.
Please remove all changes outside PExplicitRuntime codebase. They are not in scope here.
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.
Same. Please remove all changes outside PExplicitRuntime codebase. They are not in scope here.
benchmarksRuns.sh
Outdated
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.
Let's remove this file. Keep it for your local experimentation only. No need to push upstream.
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.
Shared a bunch of comments to address.
Looks like file PExplicitThreadLogger.java line 58 has a hard-coded path. That is making many CI tests fail.
|
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.
Looks like the default number of threads is set to 1. Can we try changing this to a higher number (say 4) and rerun the regression tests (i.e., mvn test
).
…s are placed are inside the P folder (else can then change) 3. For nproc 4, added code for killing other threads when bugfoundException is raised
…rror: comment and uncomment L151 and L152, and run for both
Merging on dev branch of parallel PEx (dev/pex_parallel). |
3 (out of previously 221) bugs remaining.
Code changes:
Current buggy benchmarks are: