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

Improve promote_op #17929

Merged
merged 1 commit into from
Aug 11, 2016
Merged

Improve promote_op #17929

merged 1 commit into from
Aug 11, 2016

Conversation

pabloferz
Copy link
Contributor

This is the promised fix for #17794 (at least it works locally for me).

CC @JeffBezanson, @tkelman

@tkelman
Copy link
Contributor

tkelman commented Aug 9, 2016

@nanosoldier runbenchmarks(ALL, vs = ":master")

@pabloferz
Copy link
Contributor Author

@tkelman I made a mistake and force-pushed a commit. Can we try again?

@tkelman
Copy link
Contributor

tkelman commented Aug 9, 2016

Okay, not sure exactly what this'll do. We'll also probably want to compare this against 0.5.0-rc0.

@nanosoldier runbenchmarks(ALL, vs = ":master")

@kshyatt kshyatt added types and dispatch Types, subtyping and method dispatch potential benchmark Could make a good benchmark in BaseBenchmarks labels Aug 9, 2016
@nanosoldier
Copy link
Collaborator

Something went wrong when running your job: RemoteException(2,CapturedException(NanosoldierError: error when preparing/pushing to report repo: UndefVarError(:result),Any[( in printreport at BenchmarkJob.jl:481,1),( in open at iostream.jl:113,1),( in report at BenchmarkJob.jl:387,1),( in run at BenchmarkJob.jl:233,1),( in #503 at multi.jl:1410,1),( in run_work_thunk at multi.jl:996,1),( in macro expansion at multi.jl:1410 [inlined],1),( in #502 at event.jl:46,1)]))
cc @jrevels

@jrevels
Copy link
Member

jrevels commented Aug 9, 2016

Okay, not sure exactly what this'll do.

It just adds the job to Nanosoldier's queue, so that it will run as soon as a node is free. It shouldn't mess anything up, other than possibly delivering job statuses in a weird order if the jobs happen to run concurrently with the old job (which will continue to run until completion or failure).

The error comment is Nanosoldier trying to report the build failure, but hitting a dumb typo in the error handling code that I've just fixed (some weekend I need to sit down and do a test coverage sprint for Nanosoldier...). I haven't yet deployed that fix, so error comments might be wonky for tonight, but I'll deploy it tomorrow.

@nanosoldier
Copy link
Collaborator

nanosoldier commented Aug 10, 2016

Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @jrevels

edit by tkelman: this was relative to master

@tkelman
Copy link
Contributor

tkelman commented Aug 10, 2016

@nanosoldier runbenchmarks(ALL, vs = ":release-0.5")

@tkelman tkelman closed this Aug 10, 2016
@tkelman tkelman reopened this Aug 10, 2016
@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@tkelman
Copy link
Contributor

tkelman commented Aug 10, 2016

So relative to master this is much better, relative to release-0.5 where #17389 was reverted there are just a couple of regressions and they're not too huge.

@pabloferz
Copy link
Contributor Author

The remaining regressions seem to come from somewhere else. See https://github.com/JuliaCI/BaseBenchmarkReports/blob/07527ce784fc022b854f37a29230dad4816a6895/daily_2016_8_10/report.md

@tkelman
Copy link
Contributor

tkelman commented Aug 11, 2016

Jeff, is this okay to merge?

@JeffBezanson
Copy link
Member

Yes. Thanks @pabloferz .

@JeffBezanson JeffBezanson merged commit f964e10 into JuliaLang:master Aug 11, 2016
@tkelman
Copy link
Contributor

tkelman commented Aug 11, 2016

I'll plan to backport this for RC3, along with reverting dbdaf72 and e6f5d89

@pabloferz pabloferz deleted the pz/promop branch August 11, 2016 17:42
tkelman added a commit that referenced this pull request Aug 20, 2016
tkelman added a commit that referenced this pull request Aug 20, 2016
tkelman pushed a commit that referenced this pull request Aug 20, 2016
(cherry picked from commit 6fd91b2)
ref #17929
@KristofferC KristofferC removed the potential benchmark Could make a good benchmark in BaseBenchmarks label Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants