-
Notifications
You must be signed in to change notification settings - Fork 14
Assertion builds #90
Comments
I think this is a good idea, but I would generally differentiate between CI/CD and release builds. For CI/CD since it sometimes is hard to understand why a issue occurred having assertions is very valuable and the performance cost is ok. On the other hand for official releases we shouldn't enable asserts. I would even go further and say that for CI/CD we only should have asserts builds, with downstream CI pulling those assert builds. So we should only have two builds for [release] and a single assert build for nightly, and Travis should not get a choice and use the assert builds throughout. cc: @vtjnash |
From the perspective of the buildbots I think we should still build
noassert versions so that when Eager Edward downloads the latest nightly
from JuliaLang.org, he isn’t surprised by slow performance. Or perhaps your
assertion (heh) is that the performance difference is negligible?
…On Thu, Oct 4, 2018 at 01:43 Valentin Churavy ***@***.***> wrote:
I think this is a good idea, but I would generally differentiate between
CI/CD and release builds. For CI/CD since it sometimes is hard to
understand why a issue occurred having assertions is very valuable and the
performance cost is ok. On the other hand for official releases we
shouldn't enable asserts. I would even go further and say that for CI/CD we
only should have asserts builds, with downstream CI pulling those assert
builds.
So we should only have two builds for [release] and a single assert build
for nightly, and Travis should not get a choice and use the assert builds
throughout.
cc: @vtjnash <https://github.com/vtjnash>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#90 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAH_aFXPFdakF8i4PZiolFxFKl9XV000ks5uhPcvgaJpZM4XGf1->
.
|
I do all my performance testing on the ASSERT configuration. Also asserts slow down the runtime and the compiler but not generated code. |
So perhaps what we should do is build two of both, use assert versions for all Travis/Appveyor type things, and then link to the nonassert versions from the main JuliaLang downloads page; so that things downloaded by a human tend to be fast to compile, but anything automated is properly checked. I can imagine people being upset that the version they download from the web is (measurably) slower than what they get by building from source due to assertions. |
Sure, that sounds reasonable, as long as storage doesn't get to expensive
and I am bit concerned that our windows CI build won't hold up the stress,
but we could probably add a couple of extras there.
…On Thu, Oct 4, 2018 at 10:25 AM Elliot Saba ***@***.***> wrote:
So perhaps what we should do is build two of both, use assert versions for
all Travis/Appveyor type things, and then link to the nonassert versions
from the main JuliaLang downloads page; so that things downloaded by a
human tend to be fast to compile, but anything automated is properly
checked. I can imagine people being upset that the version they download
from the web is (measurably) slower than what they get by building from
source due to assertions.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#90 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAI3ahMCAlAbfoQWfy9qvHJzKl8VWnVfks5uhhpRgaJpZM4XGf1->
.
|
While I'm generally supportive of having multiple builds available, I'm a bit skeptical of having package CI running a different Julia binary than an average user would download and use locally. I think release builds with assertions enabled on CI should be opt-in, with assertions enabled for nightly. We can make clear on the downloads page that nightly builds have slightly different settings than releases so that users have a better idea of what they're getting and what to expect. For reference, FreeBSD's master/nightly builds has a lot of debugging information enabled by default and they document that it will be slow because of this, and that you have to build it yourself with specific options to disable the things that make it slow. (That's what I do on my desktop partition.) That seems like a good model to follow for us. So I'd suggest:
While having multiple builds for each release version kind of sucks, I'd be interested to see what the difference is between having multiple binaries and the days when our binaries included a full debug build in terms of file size/storage cost. |
Closed by #111 |
Julia and LLVM have debug assertions that we can enable for better error reporting. It would be good to have PRs and pushes get put through the ringer of "assertion" builds, and also for users to be able to use assertion builds. And so here is my master plan:
We create two schedulers: the
PR
scheduler, and thepush
scheduler. They are identical, except for the fact that thepush
scheduler adds a[release]
tag, which means that this build should eventually get promoted, instead of just being tested and then left to rot. (Side note; if at all possible, we should customize our GitHub statuses so that users can download the build artifacts right there from the PR CI screen)Each scheduler kicks off two builds; one with assertions and one without. Both will get uploaded, both will get tested, both will get promoted (if they have the
[release]
tag). We then changetravis
andappveyor
to have new default versions. We'll support the following:Where
1.0
is an alias for1.0-noassert
, andnightly
is an alias fornightly-assert
. In this way, we get:Greater flexibility, by providing users the ability to opt out of asserts on nightly builds if they need to.
Greater coverage with assertions when testing both base Julia and the package ecosystem (for those packages that are already testing the waters with
nightly
, but leaving alone the poor packages that just want to make sure their single-line test suite passes on1.0
)Greater AWS storage costs because we now store every build twice.
What do you guys think? @vchuravy @iblis17 @ararslan
The text was updated successfully, but these errors were encountered: