IRC logs for #buildstream for Wednesday, 2018-10-10

gitlab-br-botbuildstream: merge request (chandan/bst-and-docker->master: Add documentation and NEWS entry about bst-docker-import) #864 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/86400:21
gitlab-br-botbuildstream: merge request (chandan/bst-and-docker->master: Add documentation and NEWS entry about bst-docker-import) #864 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/86400:59
*** xjuan has quit IRC01:22
*** catonano has joined #buildstream04:30
*** catonano has quit IRC05:01
*** catonano has joined #buildstream05:15
*** catonano has quit IRC06:43
*** tristan has joined #buildstream07:00
Kinnisontristan: Hi!  I've been investigating quite an unpleasant performance issue related to large projects and wondered what the best way to open up discussion on it would be07:47
Kinnisontristan: I could post to the ML, or else open an issue on GL.  Which would you prefer?07:48
*** ChanServ sets mode: +o tristan08:10
tristanKinnison, If there is an unpleasant performance issue, should file an issue anyway. If the approach you have in mind to solve it is invasive; propose on mailing list and link to issue. If you want to discuss the issue and get more eyes on it, also mailing list and link to issue08:10
tristanAnyway, issues are issues - we like to avoid filing issues for "tasks" or feature additions in advance of a proposal passing08:11
Kinnisontristan: understood, I'll start by filing the issue then08:11
gitlab-br-botbuildstream: issue #703 ("Queues do too much work during scheduling") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/70308:17
Kinnisontristan: the brief writeup is at https://gitlab.com/BuildStream/buildstream/issues/70308:18
Kinnisontristan: I will need to think hard about an amelioration pathway before I can recommend anything on the list08:19
Kinnisontristan: it may make sense to have a short session about it at the gathering first instead08:19
Kinnisontristan: Cooler heads than mine may come up with an elegant solution which is evading me08:19
*** rdale has joined #buildstream08:28
tristanKinnison, looking at 703, it seems quite fixable; the biggest obstacle being the crazyness of Element._update_state()08:30
*** toscalix has joined #buildstream08:31
tristanKinnison, I.e. from the first sentences, it seems clear that we are not correctly caching state, and we are redundantly interrogating the disk08:31
tristanUltimately, this state should not need reinterrogation as it should already be cached on the element in memory08:31
Kinnisontristan: I assumed reinterrogation was performed because other processes might have injected it into the cache in the meantime08:32
Kinnisontristan: regardless, the CPU consumption of calling _update_state() over the entire waiting queue repeatedly seems unnecessary if the actual state changes can be edge-propagated instead08:32
Kinnisontristan: I hope that there's a small fix which can improve matters, but I worry it might end up being quite intricate to cover all the state change cases08:33
tristanCurrently, as long as an artifact doesnt exist we will re-interrogate it; that part could be cached and propagated indeed, but sometimes we don't yet know what a cache key will be08:34
tristanBut as the build moves forward, once an artifact exists, it should no longer need any re-interrogation08:34
tristanAnd the order in which elements are processed should minimize these interrogations08:34
tristanIf we can serialize all this state from subprocesses and compute it entirely in memory in the main process, of course that will be much nicer08:35
KinnisonI think once an artifact exists, it'll pop off the end of the build queue and so not be reinterrogated08:35
tristanBut before going that far, I do suspect we are over-interrogating08:35
tristanKinnison, Whether or not it is on the queue, it will exist in the form of a dependency of another element08:36
KinnisonTrue08:36
KinnisonIf we can cause _update_state() to pop back out early if there's nothing to do, then we wouldn't need to change much else to improve matters08:36
Kinnisonit'd probably seriously improve the startup speed too because both Pipeline and Stream end up calling _update_state() in various ways across the loading and prparing stages08:37
tristanThe initial _update_state() is necessary, if it's over-working that is a problem surely08:37
tristanI'm skeptical of just how much we can save at startup for that, but at least artifact cache interrogations should be one off and not recursively redundantly doing syscalls08:38
tristanThe source cache interrogations are pretty good in this regard afaics08:38
tristani.e. Source.get_consistency() reports should be no more than needed08:39
KinnisonMmm, I accept that startup might not be too bad, just running some analyses08:40
KinnisonI think total calls to `.contains()` before the scheduler starts up might be minimal. It seems only 243 of the 13853 calls to it occur outside of `Scheduler.run()` in my test set08:42
KinnisonThough there's an obvious way to halve those if the weak and strong keys are identical08:43
KinnisonIf you think it'd be valuable, I could tar up this trace pile and put it somewhere, though frankly there's not many classes of traceback over all :-)08:44
tristanKinnison, In fact, there is indeed a way to optimize startup in that regard (but it's not our biggest bottleneck)08:57
KinnisonFair :-)08:57
tristanWe should be able to parallelize at least source interrogations08:58
tristanAnd ultimately the _update_state() stuff too, if we can serialize those results in memory in the main process08:58
tristanjust, a bit tricky to do08:58
KinnisonSounds like a future-optimisation goal.  Short-term I'd be grateful if we can find a way to shortcircuit unnecessary `Element._update_state()` calls :-)09:01
tristanWe really should be able to at least do it in the task processes :-S09:02
KinnisonThe ultimate goal would be that `*Job.status()` don't have to call `Element._update_state()`09:03
Kinnisonerm `*Queue.status()` even09:04
tristanWell, something needs to happen around there09:05
tristanBut it might be transparent for Queue subclasses09:05
tristanAnd it would mean mostly, just transmitting the result of update_state() in the sub process to the main process09:06
tristanBut, that all needs a real refactor09:06
tristanDifferent queues add to element state in different ways, and different configurations of elements handle those events differently09:06
tristanI wanted an event based approach to updating state, juergbi made good arguments at the time that handling it all in a central function was simpler in some ways09:07
KinnisonIf the element whose job has just completed has _update_state() called in the parent, and then *it* is responsible for propagating that to elements which depend on it, stopping that propagation once an element's external state is unchanged, that'd likely be enough09:07
tristan(i.e. when I say event based: I wanted to tell the element something like: Element._event(ElementEvent.BUILT), etc09:08
tristan)09:08
KinnisonYeah that's what I was thinking would be the better solution09:08
Kinnisonit's marginally more intricate to implement than the current approach, but would have better potential for doing minimum work on change09:09
tristanAlso, splitting up these state transitions into separate code blocks allows us to assert invariants more clearly, I think09:11
tristanI would like to see this organized into classed state transitions; i.e. a "workspaced element" implements a "ready -> built" state transition differently09:12
tristanAnd strict/non-strict mode also implies different handling09:13
KinnisonI think that would be valuable.  Perhaps we could whiteboard it next week?09:13
tristanLooks like we have a lot of things on the menu, but certainly a tentative yes :)09:13
Kinnison:-)09:13
KinnisonAt least it's now recorded and in your head as a performance issue rather than a "just" an "It'd be nicer if..."09:14
*** jonathanmaw has joined #buildstream09:14
tristanKinnison, heh, not me you have to convince about that09:14
tristanIf I could allocate people's time, I would concentrate on the "it'd be nicer if" architectural issues; and presume that performance benefits will be a natural result of good refactoring :)09:15
Kinnisonheh09:15
gitlab-br-botbuildstream: merge request (richardmaw/distinguish-sandboxing-build-fail->master: WIP: Distinguish between bubblewrap sandboxing failure and command failure) #868 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/86809:17
*** bochecha has joined #buildstream09:19
*** mohan43u has quit IRC09:20
KinnisonIf I wrote up a neater traceback capturing system as an MR, so that it could easily be inserted into things to do debugging, would that be welcomed, or rejected?  (I can see good reasons for the latter, so don't worry that I'd be offended if you say 'rejected')09:21
*** mohan43u has joined #buildstream09:23
tristanKinnison, There are a few things I want to do to improve debugging, I'm not sure exactly what you mean by traceback capturing system and how it would be enabled and what it would do09:31
tristanKinnison, One thing I found myself doing recently, is inserting traceback call trace prints where messages are issued09:32
Kinnisontristan: basically a tidied up form of what I did for that issue09:32
tristanOne thing I want, is to add debugging "domains" for MessageType.DEBUG, and add a way for the --debug CLI option to specify which domains to trace09:33
tristanAnd automate selection of plugins in general for debugging09:33
tristanFor instance... bst --debug "artifacts:element(compose):sandbox"09:34
tristanor bst --debug "all"09:34
tiagogomesSame style as gstreamer would be powerful :)09:34
tristanSomething like that yeah09:34
toscalixI have sent material for two of the discussion topics we will have during the Gathering so we can advance the feedback offline09:47
toscalixjjardon: I know you are a busy man but I really need your points on the Proposal Topics table for the Gathering today09:49
toscalixadds68: ^^ maybe you can do it if jjardon can't? I would like to to finish the agenda draft today09:53
jjardontoscalix: I will09:53
toscalixjjardon: thanks09:53
jjardontoscalix: points fdsk wants to talk about are already there10:03
jjardonwait I will add another10:03
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78610:11
coldtomjonathanmaw, what release number will bst-external be at with the addition of the git-tag plugin?10:15
jonathanmawcoldtom: I'd bump the version of bst-external to 0.610:16
*** abderrahim[m] has quit IRC10:16
*** pro[m] has quit IRC10:16
*** oknf[m] has quit IRC10:16
*** ssssam[m] has quit IRC10:16
*** theawless[m] has quit IRC10:16
*** inigomartinez has quit IRC10:16
*** awacheux[m] has quit IRC10:16
*** mattiasb has quit IRC10:17
*** asingh_[m] has quit IRC10:17
*** segfault3[m] has quit IRC10:17
*** tlater[m] has quit IRC10:17
*** mohan43u has quit IRC10:17
coldtomas i thought, thanks!10:18
*** mohan43u has joined #buildstream10:21
*** abderrahim[m] has joined #buildstream10:24
jonathanmawcoldtom: ah, I think there's been a misunderstanding. I meant to add the note under the "next version" heading, and I'd add the "bst-external 0.6" heading when I release the next version10:25
*** pro[m] has joined #buildstream10:25
jonathanmawit's harmless, though, so I'll merge it when CI finishes10:25
*** oknf[m] has joined #buildstream10:34
*** ssssam[m] has joined #buildstream10:40
jjardonjonathanmaw: apart frrom the governance matter in bst-external, there is another technical conversation but that is already happening at https://gitlab.com/BuildStream/buildstream/issues/69710:42
jjardonwe can touch that with tristan and thomas at any point in the gathering10:43
*** theawless[m] has joined #buildstream10:44
*** inigomartinez has joined #buildstream10:45
*** abderrahim has quit IRC10:52
*** abderrahim has joined #buildstream10:52
toscalixjjardon: mention it during the topic and make sure you schedule a slot during the hacking sessions so we keep track of it during the retrospective. Some people is going to be very busy that week. You will need to schedule in advance10:54
*** slaf_ has joined #buildstream10:58
*** slaf has quit IRC11:00
*** slaf_ is now known as slaf11:00
*** awacheux[m] has joined #buildstream11:09
jjardontoscalix: done; I have called "Extending plugins"11:13
toscalixjjardon: is this a different topic discussion that the one already scheduled or can we merge them?11:19
toscalixjjardon: this is the scheduled session https://calendar.google.com/event?action=TEMPLATE&tmeid=NTA1ODRrdm1hZDRyaDRsbGtrdWV2dDUyYWkgY29kZXRoaW5rLmNvLnVrX21wZ2FoMHVqNTM4aG5ic2Y0bDdiNHJjaHRzQGc&tmsrc=codethink.co.uk_mpgah0uj538hnbsf4l7b4rchts%40group.calendar.google.com11:19
toscalixah, forgot to add you to the participants so you receive a mail11:21
*** mattiasb has joined #buildstream11:21
*** asingh_[m] has joined #buildstream11:30
jjardontoscalix: different, but they can go together11:42
toscalixok11:43
*** segfault3[m] has joined #buildstream12:01
*** tlater[m] has joined #buildstream12:08
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78612:16
toscalixsent by mail the instruction to  register to  the training sessions12:22
* tristan thinks we should move all of _ostree.py into plugins/sources/ostree.py, now that we don't use it anywhere else12:22
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78612:41
*** cs-shadow has joined #buildstream13:00
cs-shadowtristan: hey! Quick question - did https://gitlab.com/BuildStream/buildstream/merge_requests/819#note_105468622 make sense to you or do you still think we should revert it?13:01
tristancs-shadow, The function is called Element.dependencies(), and Scope defines what is captured, some scopes do include the self element in this "list of dependencies"13:07
tristanSure the nomanclature is perhaps imperfect, but I don't think changing it is worth all the diffs clouding history there13:08
tristanthe nomanclature is not *that* bad anyway13:08
cs-shadowtristan: okay, I can revert that change. Does the rest look good to you?13:09
tristancs-shadow, I think so, honestly I would love to see the diff without those changes - as I mentioned in the comment: "It is unclear to me what has actually changed in this diff to element.py."13:10
cs-shadowsure, let me do it now13:11
tristancs-shadow, I would just say yes, but that is a sensitive function13:11
cs-shadowmakes sense13:11
tristanOverall it does make sense to me though13:11
*** lantw44 has quit IRC13:17
*** lantw44 has joined #buildstream13:17
*** lantw44 has joined #buildstream13:17
gitlab-br-botbuildstream: merge request (chandan/bst-checkout-build->master: Add `--deps build` option to `bst checkout`) #819 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/81913:20
cs-shadowtristan: updated ^. Does it look good to you now? I'll update the corresponding 1.2 branch as well if this makes sense to you13:26
tristanI'm looking now :-S13:28
cs-shadowthanks!13:29
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78613:38
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78613:43
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78613:47
gitlab-br-botbuildstream: merge request (chandan/bst-checkout-build->master: Add `--deps build` option to `bst checkout`) #819 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/81913:47
cs-shadowtristan: and now with better commit messages ^13:47
tristansec13:47
tristanstill looking13:47
cs-shadowtake your time :)13:48
tpollardsorry for the bot spam, trying to debug something that's seemingly only happening in CI :)13:52
*** catonano has joined #buildstream13:56
tristancs-shadow, I think that's it for the comments13:57
tristanyou have 313:57
toscalixyou can always mirror the repo/branch and copy the CI config in your own gitlab account13:57
toscalixso tpollard you can do all the tests you want13:57
toscalixand config the notifications to your own account13:58
cs-shadowtristan: thanks! I'll finish it up later today.13:58
tristantpollard, we don't mind the spam from the gitlab bot, don't bend over backwards :)13:58
tristanEven, if you want to avoid spam, you can always create a branch that is *not* associated with a merge request13:59
tristanActually, people love to create merge requests prematurely, and I don't really understand why13:59
tpollardAre the runners for the buildstream.git beefier ones though?14:00
tpollardi.e if I fork will I get the free ones, as the integration tests can take a while14:00
tpollardtristan: that's just the process I've been told to follow :)14:01
tristantpollard, It's alright, I don't really mind the spam, I will just occasionally criticize the early creation of merge requests :)14:04
tristanyeah, if you use a separate repo, (A) you will have all the headache of setting up CI yourself... (B) You will have all the hassle of keeping your master branch in sync with upstream master and rebasing will be really annoying (C) We will not be able to press a "Rebase" button on merge requests you submit from that repo14:05
tristantpollard, So we recommend people just ask for developer access to avoid all of that hassle14:05
tristanAnd yeah, I don't think you have permission to use our runners, either14:06
tristanWhich means CI will not have cached sources, and will take > 1hrs to complete every run14:06
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78614:16
gitlab-br-botbuildstream: merge request (willsalmon/outOfSourecBuild->master: Out of source builds) #776 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/77614:21
laurenceskullman, wrt to the patch in upstream bwrap - https://github.com/projectatomic/bubblewrap/pull/29314:28
laurenceooi, what do they mean by 'bot, add author to whitelist'?14:29
KinnisonIt's an instruction to their CI/Review bot14:29
skullmanlaurence: I'm guessing they only want to run good faith patches through CI14:29
laurenceskullman, right, i see14:30
laurenceKinnison, I knew that bit :)14:30
laurencewill v soon time to be pushing those bwrap folks for some attention14:35
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78614:36
*** tristan has quit IRC14:40
gitlab-br-botbuildstream: merge request (jonathan/debug-remote-failed-builds->master: Jonathan/debug remote failed builds) #869 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/86914:44
*** tristan has joined #buildstream15:02
*** ChanServ sets mode: +o tristan15:03
jonathanmawskullman: iirc you did some research into creating anonymous directories. Would they work in python?15:06
skullmanjonathanmaw: what exactly do you mean by an anonymous directory?15:07
jonathanmawskullman: I've hit the thorny issue of needing a "scratch" directory that's on the same filesystem as the directory I want to use as a sysroot for `bst shell`. Since it needs to be the same filesystem, I can't just use a tmpfs15:08
jonathanmawand I can't think of a good place to put it on the filesystem (can't put it in the sysroot directory, putting it in the directory above the sysroot will make it vulnerable to being messed with)15:09
skullmanif you're after something O_TMPFILE like but a directory you'll be disappointed no matter what language you use, the closest you get is that a process can have its root and current directory be a directory that is unlinked, but you can't create new files in it15:09
jonathanmaw:-/15:09
skullmanif you had the right privileges there's theoretically something you could do with a loopback mount or fuse, but that wouldn't count as being on the same filesystem15:09
skullmanon btrfs or a new enough XFS you could theoretically do zero-copy from your fuse filesystem to the destination, but that only helps with the data, not the metadata, and would be a huge amount of work15:11
tristanjonathanmaw, one way to look at this, is that the "scratch" directory is basically used by our fuse layer, and as such is important for copy-on-write hardlinks when running builds15:13
tristanjonathanmaw, so, if we have "given" the user a sysroot in some way, like via a checkout, it's questionable whether we need to do the fuse stuff at all in that case15:14
tristanbecause the user has "received" the directory, they should not have received a directory full of files hardlinked into the artifact cache, compromising the artifact cache15:14
tristanIf they use `bst checkout --hardlinks`, which has a big fat warning in the help, they are shelling into it at their own risk15:15
*** catonano has quit IRC15:49
flatmushIs it possible to build multiple elements from the same git repository?16:16
KinnisonI don't think there's any issue with multiple elements having the same source16:17
KinnisonIf, on the other hand, you mean that you have one source building multiple things and then you want to use them independently, you should look at filter elements IIRC16:17
Kinnisoni.e. have an element build everything, and then have filter elements which split up the result16:17
*** jonathanmaw has quit IRC16:38
*** toscalix has quit IRC17:19
gitlab-br-botbuildstream: merge request (patch-2->master: README: Update to add link to website) #859 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/85917:48
gitlab-br-botbuildstream: merge request (tpollard/494->master: WIP: Don't pull artifact buildtrees by default) #786 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/78617:48
gitlab-br-botbuildstream: merge request (patch-2->master: README: Update to add link to website) #859 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/85917:50
*** asingh_[m] has quit IRC18:08
*** ssssam[m] has quit IRC18:08
*** asingh_[m] has joined #buildstream18:08
*** ssssam[m] has joined #buildstream18:08
*** xjuan has joined #buildstream18:30
*** catonano has joined #buildstream18:33
*** rdale has quit IRC19:03
gitlab-br-botbuildstream: merge request (patch-2->master: README: Update to add link to website) #859 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/85919:24
*** finn has joined #buildstream19:28
*** finn has quit IRC20:23
*** tristan has quit IRC21:19
*** cs-shadow has quit IRC21:20
*** catonano has quit IRC21:44

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!