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

Backports for 0.6.4 #27621

Merged
merged 12 commits into from
Jul 4, 2018
Merged

Backports for 0.6.4 #27621

merged 12 commits into from
Jul 4, 2018

Conversation

ararslan
Copy link
Member

@ararslan ararslan commented Jun 17, 2018

Still to do:

Keno and others added 2 commits June 16, 2018 18:46
This issue possibly fixes #24951 (or at least the test case by iamed2).
We believe the original code here meant to say either:

    ((jl_typemap_entry_t*)v)->min_world = ((jl_typemap_entry_t*)v)->max_world + 1;

or

    ((jl_typemap_entry_t*)v)->max_world = ((jl_typemap_entry_t*)v)->min_world - 1;

i.e. set the range of applicable worlds to be empty. What happened instead
was that the given typemap entry that was supposed to be deleted became valid
for one particular world and that world only. Thus any code running in that
particular world may try to access the deleted typemap entry (or add a backedge
to it), causing either incorrect behavior or the assertion failure noted
in the issue. One additional complication is that these world ages are being
deserialized, i.e. they may be larger than the currently possible max world age.
This makes this slightly more likely to happen, since the current process
may work its way up to that world age and exectue some code.

In any case, there's not much value to keeping around the deserialized max or min
world, so just mark them as [1:0], as we do for other deleted entries.

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

---

NOTE: This backported commit EXCLUDES additional assertions made by
vtjnash.

(Cherry-picked from commit d9b10f0)
@ararslan ararslan added this to the 0.6.x milestone Jun 17, 2018
by avoiding calling zero(T). Also a few minor performance improvements.

(Cherry-picked from commit 3fa83f9)
@ronisbr
Copy link
Member

ronisbr commented Jun 18, 2018

@ararslan any chance to include #27406 in backport list so that the tests in openSUSE can finally pass?

Currently we're putting build directories as well as GCC's directory
directly into our libraries' RPATHs. This is fine for simple source
builds that sit where they were built and aren't distributed, like on
CI. However, this causes major issues for distributing.

Ref #27396
(cherry picked from commit 5a426b0)
First of all, we need to add libgfortran, libgcc_s, and libquadmath to
`JL_PRIVATE_LIBS-0` instead of `JL_LIBS`. This ensures that they get
installed into `lib/julia`, where our dependencies are supposed to live,
instead of `lib`, where only libjulia should live.

Now we need to call `fixup-rpath.sh` on more directories. Currently
we're only calling it on `lib`, but we also need to call it on
`lib/julia` and even on `bin`, because FreeBSD seems to load libraries
based on the executable's RPATH rather than libjulia's RPATH.

AND, speaking of the executable's RPATH, we need to make sure we have
`lib/julia` in there; currently we only have `lib`. So in an isolated
environment outside of the build directory, Julia won't be able to load
any of its dependencies, which is bad. This is accomplished with a
simple adjustment in `Make.inc`.

To make sure that our dependencies can find each other properly, we need
to add `RPATH_ESCAPED_ORIGIN` to `LDFLAGS` in `CONFIGURE_COMMON`. That
way, every dependency that uses `configure` will get its RPATH set
appropriately.

(cherry picked from commit 07edead)
ronisbr and others added 2 commits June 18, 2018 16:30
This fixes an error in tests when libgit2 version is equal or newer
v0.26.0.

Closes #24453

Ref #27406
This reverts commit 3a27e01.
As noted in #27652, this change is breaking.
@ararslan
Copy link
Member Author

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

@nanosoldier
Copy link
Collaborator

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

@garrison
Copy link
Member

Any chance this can be called v0.6.4-rc1 once it is merged? Then people following release-0.6 can test for a few days and identify any potential regressions before v0.6.4 final. It seems to me that this could have been helpful with ironing regressions out of v0.6.3, but please treat it as a suggestion rather than a request.

@StefanKarpinski
Copy link
Member

Can you build and test the release-0.6 branch locally instead? Doing release candidates for point releases seems like a bit much.

@garrison
Copy link
Member

To be clear, I don't care whether it gets a special name, binaries, or even a tag; my only real suggestion is that there actually be some time for testing between when this is merged into release-0.6 and when v0.6.4 is tagged. Looking at git history, it seems most recent releases other than v0.6.3 have had such a testing period. As I said before, it's really just a suggestion; I'll leave to Alex how to run the release process.

@ararslan
Copy link
Member Author

it seems most recent releases other than v0.6.3 have had such a testing period

Where do you see that?

@garrison
Copy link
Member

A person following the release-0.6 branch during the v0.6.3 cycle would have seen only the following changes:

  • 93168a6 Set version to 0.6.3-pre
  • 41143e8 change PCRE download link
  • 261e488 massive merge of backports
  • d55cadc v0.6.3 final release, less than 24 hours later

I don't see another release on the 0.6 cycle where so many backports were merged right before tagging a final release. v0.6.0 could be considered an exception, but there were rc's for it. v0.6.1 had a commit just before it was released, but the vast majority of the backports had been sitting on the release-0.6 branch for weeks at that point.

@ararslan
Copy link
Member Author

the vast majority of the backports had been sitting on the release-0.6 branch for weeks at that point

If I recall that's only because we were having issues with the buildbots.

some lookup functions (most notably, jl_get_specialization1)
were bypassing the initial cache lookup,
resulting in repeatedly trying to re-cache them,
and wasting a small amount memory

also remove some fast-path code for threading race conditions:
the benefit is too negligible to be worth the extra code maintenance

(cherry-pick of e36ad5c, PR #26982)
@ViralBShah
Copy link
Member

ViralBShah commented Jun 23, 2018

I think it is ok to merge the commits, and announce a 3 day testing period before tagging - but RCs and such would be way too much for point releases.

Going forward, we can try to have a week of testing between merging all the commits and tagging/releasing.

@StefanKarpinski
Copy link
Member

announce a 3 day testing period before tagging

This is a good idea. Merging all the commits all at once and then tagging really doesn't give any chance for feedback.

@nalimilan
Copy link
Member

nalimilan commented Jun 24, 2018

Also, post-1.0, a Discourse post telling people to test the branch before publishing minor releases, with links to nightlies, would be useful. We really don't want to discover regressions a few days after the release.

@ronisbr
Copy link
Member

ronisbr commented Jun 25, 2018

It will be very fine to have a testing period. Furthermore, is it too difficult to set a nightly build of release-0.6 branch? In this case, it will be easier for people to test, since Julia takes way too much time to build on slow machines.

@ViralBShah
Copy link
Member

Should be possible to set up nightlies whenever we have adequate time - post 1.0. For now, the easiest thing to do would be to merge this PR, trigger a build that prepares the binaries and if that works out ok, tag and create new binaries.

Of course this adds much more effort on @ararslan 's part, so he should chime in if that would be onerous.

@iamed2
Copy link
Contributor

iamed2 commented Jun 26, 2018

We keep running into #27568. What is holding this up?

@ararslan
Copy link
Member Author

is it too difficult to set a nightly build of release-0.6 branch?

Interesting idea. That's not difficult, per se, but it seems rather unnecessary to me. By design, the release branch should be entirely static with the exception of backports which happen right before the release. An easier solution would be to have testing binaries built from the release branch and available for developer testing just prior to the release, like a pseudo release candidate. The downside to that is that it's a bit of a pain in the neck for the sake of short-term, one-time, non-release testing.

What is holding this up?

In short, PackageEvaluator. Once I've verified that this is non-breaking, this can be merged.

In long, PackageEvaluator seems to be running into mysterious errors with the virtual machines. PackageEvaluator separates the packages for each version into three VMs for testing and runs them simultaneously for a total of six VMs. When I initially ran it, I noticed that a huge number of packages hadn't been tested at all, and found that it was because a few of the VMs died. I thought, "Okay, maybe it's an issue with where I'm running it," so I switched to Nanosoldier node 1. But just now I saw that another VM has died. Once this run completes, I think I'll have to just finish testing for the remaining packages by hand.

@nalimilan
Copy link
Member

An easier solution would be to have testing binaries built from the release branch and available for developer testing just prior to the release, like a pseudo release candidate. The downside to that is that it's a bit of a pain in the neck for the sake of short-term, one-time, non-release testing.

That would definitely make sense. Minor releases are not "non-release", they are even more sensitive than major releases since any bug can break a workflow when people don't expect it. They should be tested even more than major releases, and we should try to get as much user testing as we can before releasing them.

@ViralBShah
Copy link
Member

I wonder if @Keno 's new Package evaluator is ready for a bit more usage.

@Keno
Copy link
Member

Keno commented Jun 26, 2018

It is, and we're using it for 0.7, but it doesn't do 0.6 for a variety of reasons.

@evetion
Copy link
Member

evetion commented Jun 29, 2018

Would it be possible to include #22886? See issue #22832.
That would greatly help us (@visr), as we now backport it ourselves.

edit: Specfic commits are 6cc10fb and 04456b9

@ararslan
Copy link
Member Author

Unfortunately that hasn't been backported because it doesn't have tests. If tests are added then we can.

@evetion
Copy link
Member

evetion commented Jun 29, 2018

The test was disabled in the first commit 6cc10fb, but it has recently been enabled and merged in 04456b9.

@ararslan
Copy link
Member Author

ararslan commented Jul 2, 2018

Oh fantastic, I hadn't seen that! I'll get that included here.

vtjnash and others added 2 commits July 1, 2018 19:29
when calling uv_shutdown on a handle being written to by another process
we might never get the UV__POLLOUT notification
but we also don't need to delay the uv_close call
if we have nothing to write

in the future, we should introduce a new `uv_drain` callback,
instead of continuing to abuse uv_shutdown for this purpose

fix #22832

Ref #22886
(cherry picked from commit 8949a95)
let's hope it passes and doesn't just hang...

(cherry picked from commit 04456b9)
@ararslan
Copy link
Member Author

ararslan commented Jul 3, 2018

Good news!

  1. Tests are passing on CI.

  2. I was able to get a PackageEvaluator run through (thanks @Keno!) and it reported no breakages. There were two failures on 0.6.4, in Interact and OdsIO, but I investigated them manually and couldn't reproduce.

I'll do one more Nanosoldier run for good measure, but I don't expect anything to change. So this should be good to go.

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

@ararslan ararslan changed the title WIP: Backports for 0.6.4 Backports for 0.6.4 Jul 3, 2018
* Fix exp10 fallback and exp2 and exp10 for Float16

* Update docstring and test

* Change float struct name

* Update math.jl

(cherry picked from commit 3ff6eac)
@ararslan
Copy link
Member Author

ararslan commented Jul 4, 2018

I really wanted this to be frozen, but I've backported one small, non-distruptive change that will fix a stack overflow for exp10. (Thanks @johnfgibson)

@nanosoldier
Copy link
Collaborator

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

@johnfgibson
Copy link
Contributor

Thanks for taking the late submission. I had no idea 0.6.4 was so close.

@ararslan ararslan merged commit b72bc45 into release-0.6 Jul 4, 2018
@ararslan ararslan deleted the aa/backports-0.6.4 branch July 4, 2018 17:37
@ararslan
Copy link
Member Author

ararslan commented Jul 4, 2018

Here we go, folks. I'll create the 0.6.4 tag on Monday, 9 July, and move forward with making binaries. In the meantime, please build the release-0.6 branch and test it out.

@ViralBShah
Copy link
Member

Should we hand over this commit to the buildbots and get nightlies?

@ViralBShah
Copy link
Member

I assume that the test period has gone well, and that silence here is a good thing.

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.