-
Notifications
You must be signed in to change notification settings - Fork 7
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
Find successful passes with genetic algorithm #900
Conversation
@nmdis1999 Does this carry over the comments I made on the other PR? |
@ltratt All but one, I have to change while loop (with |
@ltratt I am capturing timing information in |
A bit of explanation how genetic algorithm is assigning weight to list of passes. Genetic algorithm first generates a list of passes which is then used to re-compile/build YK and run YKLUA test via Since
|
@nmdis1999 If/when this has successfully found some passes, please let us know. |
@ltratt I ran the algorithm for 100 generation (and population size * 2), this ran between 4-5 days for both PRELINK and POSTLINK pass list. PRELINK
POSTLINK
|
Wow that's a lot of postlink passes! Does it make much difference to interpreter performance? |
This is looking good. All of my comments are minor. |
We're getting there! @nmdis1999 Please have a quick look over the open chats (including one or two from last year). With luck we can get this moving soon (once we have some rough numbers from your benchmarking run). |
@ltratt addressed your comments. Ready for review/ |
OK, so code review looks reasonable -- all we need to know now is whether it works! So we'll need performance numbers in here as and when you have them. |
6acd1bb
to
3e9e7cb
Compare
@nmdis1999 Please undo the force push (ironically you'll need to force push in order to do that!). |
2152eb3
to
0edbc0d
Compare
PR is ready for review @ltratt @vext01 Benchmark numbers (for 30 runs with 95% confidence interval):
|
@nmdis1999 Quick questions:
|
Sorry, I forgot to change the headers. They are all CI.
WITHOUT YKLLVM means the lua binary is compiled with O2/O3 without YK related flags.
Best Prelink set of passes:
Best Postlink set of passes:
|
tests/Cargo.toml
Outdated
@@ -36,7 +36,7 @@ yktracec = { path = "../yktracec", features = ["yk_testing"] } | |||
|
|||
[dev-dependencies] | |||
criterion = { version = "0.5.1", features = ["html_reports"] } | |||
lang_tester = "0.7.4" | |||
lang_tester = { version = "0.7.4", path = "../../lang_tester" } |
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.
You'll need to make this point to the branch in Edd's github fork. Cargo has the settings you need to do this.
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.
Hold on, we don't want to merge that into yk/master
. It fudges lang_tester to not match stdin/stdout.
What's the plan here?
ykrt/pass_finder/cargo_run.py
Outdated
for _ in range(n): | ||
subprocess.run(["make clean"], shell=True, env=env) | ||
c = subprocess.run(["make && timeout 30 sh test.sh"], shell=True, env=env or os.environ) | ||
os.chdir(yk_path) | ||
if c.returncode == 0: | ||
r = subprocess.run(["timeout 60 cargo test"], shell=True, env=env or os.environ) | ||
r = subprocess.run(f"timeout 60 cargo test", shell=True, env=env or os.environ) |
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.
The extra space after =
is unneccessary.
ykrt/pass_finder/setup_genetic.py
Outdated
def setup(curr_dir, base_temp_dir, yk_path, yklua): | ||
num_cores = multiprocessing.cpu_count() - 1 | ||
directories = [] | ||
# for i in range(num_cores): |
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.
Commented code.
f42221a is going to be a problem: we can't commit the PR with those changes, because they break "normal" yk. @vext01 do you have any thoughts on how we could handle this in the master branch? It might be that we end up having to do this stuff in a branch/fork for a while? [I'd prefer to merge into master, but aware that it might be tricky.] |
I suppose we could use a cargo feature for the kludged lang_tester? |
It's not just lang_tester though: notice that this branch also disables a test and disables code in ykcapi. |
I suppose that could be feature gated too... I'm not really sure what the best way forward here is. gate or branch... |
Please force push a rebase + squash (doesn't have to be to 1 commit though) against master. |
06f62b4
to
c3bf5ac
Compare
c3bf5ac
to
c99cd35
Compare
@ltratt rebased and pushed |
This PR aims to find successful passes (i.e which pass all YK and YKLUA tests) with the help of genetic algorithm. The fitness function currently gives priority to execution time (the lesser the better) to pick up most likely parents.