On Mon, Mar 4, 2013 at 3:41 AM, Simon King <simon.k...@uni-jena.de> wrote: > Hi Jeroen, > > On 2013-03-04, Jeroen Demeyer <jdeme...@cage.ugent.be> wrote: >> On 2013-03-04 12:01, Simon King wrote: >>> +With 99.3% confidence, startup time decreased by at least 0.1% >>> +With 99.4% confidence, startup time increased by at least 0.1% >> >>> Since the code didn't change in between, how significant is it then? >> Are you really really sure that the code didn't change? Because if it >> really didn't change, I would indeed consider the startup_time plugin >> broken (this also shows how hard it will be to do #12720 right). > > In comment 6, I first see the second patch mentioned, namely: > "Apply trac_14214-cython_homset.patch trac_14214-backup_cache.patch" > Later, the second patch was changed precisely once, namely in comment 8. > > The change of the patch, namely removal of an example, as explained in > comment 7. > > The patchbot situation is: > - The current topmost patchbot report is openSUSE/.../jehova. It seems > that it is still using the old version of the second patch, as there > is a mismatch. Here, the startup_time plugin complains with 90% > confidence about a 0.1% increase. > - Fedora/.../volker-dektop.stp.dias.ie seems to have the current patch > version. The startup_plugin complains with 98% confidence about a > 0.25% increase. > - Fedora/,../volker-desktop.stp.dias.ie also has a report with a > mismatch in the second patch (hence, it is supposed to be the same > patch as in openSUSE/.../jehova!). The startup_time plugin reports > with 97% confidence about a 0.25% *decrease*. > > I don't know if we can find out "post mortem" what patches were really > applied.
If patches are replaced, rather than new ones uploaded, then we loose a record of them. The patchbot computes the md5, you if you can hunt the original patch down somewhere we could verify what changed. > I can only conclude indirectly: The second patch saw precisely > two versions, hence, a "mismatch" means that the patchbot was using the > old version, and (see above) with *the same* old patch version one patchbot > reports a speed-up and another patchbot reports a slow-down. Exactly. > And one patchbot reports a speed-up with the old patch but a slow-down > with the new patch. But I am sure that the only change consisted in > removing a test from sage/modules/vector_space_homspace.py Getting accurate measurements on the performance of something like Sage startup is really hard... What we're seeing here is that Sage was started several times with and without this set of changes, a difference was noted, and the probability of this difference being due to bad luck is (in this case) slim. It can't really rule out other external factors though. This blue dot, like all other plugins, is a signal that should be taken into account when reviewing a patch, but is not necessarily a blocker. At the very least, it merits investigation and discussion, but in some cases that's all. The only way I could see making this foolproof is (1) starting up Sage so many times that it becomes prohibitively expensive to run the bot or (2) reducing the false positive rate so low that this plugin never fails, even when it should. If anyone has other ideas on how to better handle this, I'm all ears, but just because we can't do it perfectly doesn't mean we shouldn't try. For a ticket like this, where potential performance implications are widespread and hard to measure, it could make a lot of sense to run this (or other timing) code on an unloaded system to gather many, many more samples. - Robert -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel?hl=en. For more options, visit https://groups.google.com/groups/opt_out.
