/srv/irclogs.ubuntu.com/2018/11/21/#snappy.txt

mupPR snapd#6176 closed: snap: check max description length in validate <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/6176>00:05
mvozyga: hey, 5845 is ready for a re-review (you commented some days ago there)06:17
mborzeckimorning06:19
mvohey mborzecki06:21
mborzeckimvo: how are the tests looking today? still mostly red?06:21
mvomborzecki: I saw a green one06:22
mvomborzecki: 5845 which also needs a second review06:22
mvomborzecki: but also some reds :(06:22
mvomborzecki: I just retriggered 6156, that should tell us about the testing weather today06:22
mborzeckiheh :)06:22
zygao/06:56
zygamvo: looking06:56
zyga1K wow06:57
mvozyga: 1k?07:00
zygaabout 1K of diff07:00
zygabut no worries, reading07:01
mupPR snapd#6178 opened: snap,snap-exec: add SNAP_DEBUG_START_TIME environment <Created by mvo5> <https://github.com/snapcore/snapd/pull/6178>07:11
mvozyga: heh, things look pretty good with snap_debug_start_time set, it seems we take only ~0.1sec for brave to setup our stuff07:24
zyganice07:24
zygaso what is slow?07:24
mvozyga: I think the wrappers, I'm on it, not sure conclusively yet07:25
zygasuper07:25
mvozyga: slowly working my way up :)07:25
zygaI will finish this review, garden my branches and work more on perf monitoring07:25
zygawhile not immediately helpful to your cause it will allow us to do this more or less automatically down the line07:26
mborzeckimvo: wonder if something like snap run --strace='--raw -e execve -r' <snap.app> would work07:31
mvomborzecki: hm, good idea, maybe no need for a special tool.07:32
mborzeckilooks a bit noisy for gnome-calculator07:32
mvomborzecki: heh, its even worse with brave :)07:32
mvomborzecki: forkstat is what I'm currently using07:32
mborzeckimvo: well, with gnome calc, there's clearly lots of stuff happening07:34
mvomborzecki: yeah, tons07:34
mvomborzecki: including update-mime-database which is ~2s here07:34
mborzeckimvo: gnome-calcualtor https://paste.fedoraproject.org/paste/rCbAYtyL02YLJ9xm~-pTZg/raw07:36
mborzeckiheh, funny how not using absolute path makes multiple execve() calls07:37
zygamvo: reviewed07:39
mborzeckihm xdg-user-dirs update takes less time than head -c0 calls07:39
mvozyga: ta07:40
zygamvo: I'm sorry that it is not a +1 yet07:40
zygalet me know if you want to talk about any of this07:41
mvozyga: no worries, thanks for the review, the comments look really useful07:47
mborzeckimvo: on my machine, each call to mkdir/ln/ls/head/date takes 15ms+ (~90 calls), glib-compile-schemas ~100ms, gdk-query-pixbus-loaders ~700ms, update-icon-caches is called twice ~30ms total07:50
mborzeckimvo: that's for gnome-calculator07:50
zygamborzecki: that's all in shell, right?07:51
mborzeckimvo: yes07:52
zygaa shell program making those calls?07:52
mvomborzecki: whats the total overhead for you?07:52
mborzeckimvo: ~2s07:54
mborzeckimvo: but that's ssd, multi core and so on :P07:54
mborzeckimvo: i'll try with dropped caches07:55
=== pstolowski|afk is now known as pstolowski
pstolowskimornings07:58
zyga(mac chime)08:00
zygahey pawel08:00
pstolowski:)08:00
pstolowskican anyone make a quick test and confirm if this trivial change https://pastebin.ubuntu.com/p/zSbp4VKkgq/  causes massive mem leak and oom with go test inside snap/ dir (make sure you're ready to stop it before your desktop gets unresponsive)08:04
mvomborzecki: ta, I see ~2s here as well (also multi-core, ssd etc)08:04
mvopstolowski: good morning!08:05
zygayep, checking08:05
mborzeckimvo: hm dropped caches, removed $SNAP_USER_DATA and so on, g-c takes visibly longer to start, but it's not reflected in desktop helpers, the change there is minimal08:14
zygaSo what is the slow part?08:15
mborzeckimvo: https://paste.fedoraproject.org/paste/w-X6E2U7OKjM9n0210BGTA see line 644, there's a bunch of processes started, but they don't exec08:15
zygaApart from fonts08:15
mborzeckimvo: some glib/gtk thing?08:15
zygaTo compute 2+2 you need to fork 100 times08:17
mborzeckig_async_*08:19
mborzeckimaybe is spawning gjs under the hood :)08:19
mvomborzecki: looking08:21
mvomborzecki: why does a pastebin require 7 script to work - oh well08:21
mvomborzecki: hard to say but I would not be surprised if that was some sort of gjs08:21
pstolowskizyga: any findings?08:34
pstolowskimborzecki: you landed some improvements for diffing on failing go tests some time ago; what's the approximate location of this diffing code?08:41
mborzeckipstolowski: that'd be in check.v108:41
mborzeckipstolowski: checkers.go, there's formatUnequal()08:42
pstolowskimborzecki: got it, thanks!08:43
pstolowskimborzecki: heh, it seems it's related to the leak i'm seeing08:44
pstolowskimborzecki: i removed that entire logic08:44
pstolowskimborzecki: getting just a few test failures now (expected)08:44
mborzeckipstolowski: is that in interfaces?08:44
mborzeckii recall getting oom when something went awry there08:44
pstolowskiit in snap/ , yaml parsing tests08:45
pstolowskimborzecki: it eats mem pretty quickly with the diff i pasted earlier08:45
mborzeckipstolowski: iirc it didn't like when structs were pointing to each other08:45
pstolowskimborzecki: yep, i saw something suspicious yesterday with more complex changes (it would output "CYCLIC REFERENCE"). but in this simple case i created it's actually less references (i'm not creating slot <-> hook references)08:46
pstolowskimborzecki: in other words i deliberately broke code not to create references, and it causes oom :/08:47
mborzeckizyga: mvo: i think g-c is actually pulling some exchange rate data from the network08:47
mvomborzecki: I wrote a small script to gather the strace datahttps://paste.ubuntu.com/p/vD2rsyXHJr/08:48
mvo 08:48
mvomborzecki: https://paste.ubuntu.com/p/vD2rsyXHJr/08:48
mvomborzecki: this is from brave, I will post the script in a sec08:48
pstolowskimborzecki: i suppose the problem is actually in pretty?08:48
mborzeckimvo: nice08:48
mborzeckipstolowski: yes, that's quite possible08:49
zygapstolowski: re, sorry for the lag - some house stuff08:57
zygapstolowski: looking again08:58
pstolowskinp08:58
zygapstolowski: ah so check.v1 bug?08:58
pstolowskizyga: it seems so08:58
pstolowskizyga: or rather - it's dependency that's doing the diffing08:59
mborzeckipstolowski: keep in mind that the print only happens if the values are not equal in the tests09:02
pstolowskimborzecki: sure09:03
pstolowskimborzecki: only it's impossible to find out what's failing ;)09:03
pstolowskimborzecki: for now i just disabled this pretty printing09:04
mborzeckipstolowski: i can relate :) this was super annoying when working on interfaces09:04
mupPR snapd#6172 closed: Send epochs followup <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/6172>09:05
mborzeckimvo: my breakdown for brave, 7.6s from s-c to exec'ing brave, top 3 are update-mime-database (2.9s), gtk-update-icon-cache (1.1s), snap-update-ns (~700ms apparently)09:17
mborzeckisuprising how far one can go solely with emacs :)09:17
zygamborzecki: I think I should measure snap-update-ns09:17
zygacan you please tell me what is the mount profile there09:17
zygaboth system and user09:17
mvomborzecki: aha, ok09:18
mvomborzecki: http://paste.ubuntu.com/p/Gn95x59tqT/ <- this is what I used09:19
mborzeckizyga: http://paste.ubuntu.com/p/tg5VcSyJw8/09:19
mvomborzecki: I will include it in my bench script to make reproducing this easier09:19
mborzeckimvo: great09:20
zygathanks09:25
zygamborzecki: is that all?09:26
zygathat's the system profile, is there a user profile too?09:26
zygamaybe there's a bug but I don't see why this should take 700ms09:27
zygawell09:27
zygasomething to check09:27
mborzeckizyga: https://paste.ubuntu.com/p/wG63T4jQjd/09:27
zygaah09:27
zygaso yes09:27
zygaportals09:27
zygaI bet a beer it is portal startup check09:27
zygathough09:27
* zyga thinks09:27
zygaperhaps not09:27
zygathat was added to snap-run IIRC09:28
zygamborzecki: when you measured, 700 was for which of the two executions of snap-update-ns?09:28
mborzeckizyga: regular one, let me grab that part of strace09:29
zygawith roughly 30 mount calls there that's ~~ 20 ms per entry09:30
zyga(aka looooooots)09:31
mborzeckizyga: https://paste.ubuntu.com/p/mR7vpYFRgS/09:33
mborzeckizyga: hm, it'd be easier if snap-update-ns printed it's pid :)09:34
zygais that [pid 20700]      0.674152 [????????????????] +++ exited with 0 +++09:35
zyga0.67 is the duration?09:35
mborzeckizyga: yes, that's my guess09:36
mvomborzecki: I started using "-ttt" to get more reliable timing09:39
mupPR snapd#6179 opened: snap: epoch lists must contain no duplicate entries <Created by chipaca> <https://github.com/snapcore/snapd/pull/6179>09:39
mborzeckimvo: yeah, seems to give quite different results when tracking many syscalls09:47
mborzeckizyga: with -ttt s-u-n seems to take ~100+ ms for snap fstab and ~10ms for user mounts https://paste.ubuntu.com/p/cp7cs6jvFC/09:49
zygawhat is -ttt?09:49
zygagiven that this x7 difference is the remaining allocation accurate?09:50
zyga(to other programs in the chain)09:50
kjackalHi snappy people, is there a way to clean a channel so that it shows nothing is published there? I want to have nothing released for microk8s 1.10/edge09:51
mborzeckizyga: -t is wall clock, -r is relative time between syscalls (supposedly using montonic clock)09:51
mborzeckizyga: also, from the point where sc_fork_helper is called to where sigchld is received it's ~650ms09:52
zygathat's ok09:53
zygasc_fork_helper lives for the duration of both s-u-n calls09:53
zygain between we do our business and call sun twice09:53
mborzeckihm actually intersting, between calling sc_fork_helper and sc_populate_mount_ns there's 500ms difference, not that it matters much09:55
zygammmmmmm09:55
zygathat's interesting09:55
zyganote09:55
zygawhich version is this09:55
zygathat code changed recently09:55
zygais this master?09:55
mborzeckii've built s-u-n from master btw09:56
zygaah sorry09:56
zygamaster is "old"09:56
zygaI was thinking about my recent changes09:56
zygaso09:56
zygahmmm09:56
zygamaybe the slow part is aa change hat?09:57
mborzeckizyga: lines 8 and 15 in that last paste09:57
zygacan you do an experiment09:57
zygacomment that out09:57
zygaand see what happens09:57
zygayou may need to splice the helper hat profile into the main profile to avoid denials09:57
zygait'd be fun if that small operation took this long09:57
* zyga is happy snap-confine is in C an can be timed reliably with symbols09:58
mupPR snapcraft#2409 closed: python plugin: process deps before and separately from setup.py <Created by sergiusens> <Merged by sergiusens> <https://github.com/snapcore/snapcraft/pull/2409>10:05
=== alan_g is now known as alan_g_
zygabrb10:33
mupPR snapd#6156 closed: wrappers: remove all desktop files from a snap on removal <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/6156>10:34
mupPR snapd#6180 opened: snap/info: bind global plugs/slots to implicit hooks <Complex> <Created by stolowski> <https://github.com/snapcore/snapd/pull/6180>11:09
* pstolowski lunch11:21
mborzeckimvo: updated https://gist.github.com/bboozzoo/d4b142229b1915ef7cc0cf8593599ad9 added content verification and variants with systemd units loaded before start/stop, with systemd-mount and just simple mount11:47
mborzeckimvo: systemd-mount is flaky, so it fails early when stopping, mounts are so so, updating the state of loopback devices seem to take a while; loading mount units before reload makes the problem disappear11:48
mborzeckimvo: so the only path where it fails reliably is when reload interleaves with start (and maybe stop)11:49
mborzeckiChipaca: have you seen https://forum.snapcraft.io/t/display-non-autoconnect-interfaces-at-install/8609 ? sounds like a nice ux improvement11:52
mvomborzecki: nice findings"!11:57
mvomborzecki: this also means we can serialize daemon reload and the problem hopefully goes away?11:57
mvoGRL (GLOBAL RELOAD LOCK)11:57
mupPR snapcraft#2412 opened: schema: add support for title <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2412>12:14
cachiozyga, https://paste.ubuntu.com/p/j7BdJ6jhJJ/12:21
cachioany idea what could cause that?12:21
cachioit is using core from edge12:21
cachiomvo, ~12:21
zygayes12:21
zygabut no idea why12:21
zygawhat distribution and kernel is this?12:22
cachiobionic12:22
cachio4.15.0-36-generic12:22
zygamborzecki: ^12:22
zygais the calc open now?12:22
mborzeckihah, intersting12:23
mborzeckizyga: does edge have the fixes you landed recently?12:23
zygamborzecki: yes12:23
cachiono12:23
zygawell12:23
zygaI suspect so12:23
mborzeckiyes/no/maybe :)12:23
zygabut in either case those are not related12:24
zyganone affect discarding12:24
mborzeckimaybe gnome-calculator is still working?12:24
cachioit is not working now12:25
cachiono process running at least12:25
zygacan you look at12:26
zygacd /sys/fs/cgroup/freezer12:26
cachioI tried another snap12:26
cachiosupercalc-snap                                   1     stable    keshavnrj   disabled,broken12:26
cachioand also failed12:26
zygathen cd to snap.gnome-calculator12:26
zygaand look at cgroup.procs12:26
zygacheck what processes are there12:26
cachiozyga, I dont see calculator12:28
zygawhat do you see12:28
cachiozyga, other gnome snapd12:28
cachiosnaps12:28
zygacan you run12:29
cachiologs, characters12:29
zygasudo SNAPD_DEBUG=1 /usr/lib/snapd/snap-discard-ns gnome-calculator12:29
cachiozyga, what I see12:30
cachiois that if I remove from command line a snap12:30
cachioit works12:30
cachioif I remove from ubuntu-software the snap fails to be removed12:30
cachiozyga, https://paste.ubuntu.com/p/cNHpvdCqMM/12:31
zygathat's the one from the distro, can you run the one from core12:32
zygathat will be representative of what snapd runs12:32
zygait has more stuff going on12:32
zygasorry, I didn't think about this12:33
cachiozyga, I already tried removing from core and it works12:33
zyga?12:33
zygaI mean12:33
zygacan you try running snap-discard-ns like you did but use /snap/core/current/usr/lib/snapd/snap-discard-ns instead12:33
cachioyes12:34
zygaand capturing the log if that12:34
zygawhen it fails12:34
cachiohttps://paste.ubuntu.com/p/9Ng8pfysj9/12:35
cachiofails12:35
zygaaha12:36
zygaI suspect I know what the problem is12:36
zygathis is fixed (I suspect) by ...12:36
zygahttps://github.com/snapcore/snapd/pull/615912:36
mupPR #6159: cmd/snap-confine: handle mounted shared /run/snapd/ns <Created by zyga> <https://github.com/snapcore/snapd/pull/6159>12:37
zygacachio: can you post your host's mountinfo table12:37
zygawhat I don't know is how that can happen12:37
zygaseems like another bug12:37
Chipacamborzecki: yes i saw that12:37
Chipacamborzecki: and yes i agree12:37
mborzeckiChipaca: we should have a papercuts lane in trello12:37
Chipacamborzecki: my plate is full but i've got my eyes on it in case nobody picks it up before i finish the smallest pea12:37
Chipacamborzecki: +2312:38
zyganote how we never unmounted the file12:38
* Chipaca accidentally burned through all his +1 reserves12:38
zygajust unliked it12:38
zygathat is because it's a tmpfs12:38
zygabecause we re-mounted /run/snapd/ns over itself12:38
zygahiding mounts inside12:38
zyganot sure why12:38
cachiozyga, https://paste.ubuntu.com/p/H5sCQkVWkm/12:38
zygacan you get the mountinfo table instead12:38
zygamounts shows partial data12:39
zygabut yeah, I suspect that's the case12:40
zygatmpfs /run/snapd/ns tmpfs rw,nosuid,noexec,relatime,size=1222540k,mode=755 0 012:40
zygatmpfs /run/snapd/ns tmpfs rw,nosuid,noexec,relatime,size=1222540k,mode=755 0 012:40
cachiozyga, sorry, this is what you need https://paste.ubuntu.com/p/YrDVdcNyQV/?12:44
cachiozyga, sorry, this is what you need https://paste.ubuntu.com/p/YrDVdcNyQV/ ?12:44
zygano12:45
zygaplease cat /proc/self/mountinfo12:45
zygabrb, lunch12:45
zyga(not urgent)12:45
zygacachio: I think the bug is fixed in that PR I linked to12:45
cachiozyga, https://paste.ubuntu.com/p/Fhb8GrpJnM/12:45
zygathanks, I'll check soon]12:46
cachiozyga, ok, but why if I do snap remove foo12:46
cachioworks12:46
cachioand if I remove from ubuntu software it fails?12:46
Chipacamborzecki: trello says it isn't for papercut kind of thing12:49
Chipacamborzecki: so, https://forum.snapcraft.io/tags/papercut12:49
Chipacamborzecki: (in "about the snapd team board", no backlog and no quick tasks)12:50
mborzeckiChipaca: ah, fair enough12:54
mborzeckizyga: https://github.com/systemd/systemd/issues/1087212:55
zygamborzecki: nice13:07
zygamborzecki: I'm not 100% sure it's systemd but let's see13:07
mborzeckizyga: couldn't reproduce it any other way13:07
zygacachio: it depends on the order in the info table13:07
zygamborzecki: I will look again later13:07
zygamborzecki: need to wrap up and wrap up and do more tasks13:07
cachiozyga, ahh, ok13:09
zygamvo: I now have collection of performance events from snap-update-ns13:29
zygadetailed actions as well13:29
zyganow looking at C13:29
mupPR # closed: core-build#11, core-build#22, core-build#26, core-build#3713:30
mupPR # opened: core-build#11, core-build#22, core-build#26, core-build#3713:31
* Chipaca hugs mup13:32
Chipacasome days are like that13:32
mvomborzecki: super nice13:44
mvozyga: cool, thanks. what times are we talking about?13:44
mvozyga: I mean, how much time does snap-update-ns take on a typical snap?13:44
mborzeckimvo: zyga: it'd be nice to have a baseline reference, eg. hello-world and then the snap with all the interfaces13:47
zyganice idea13:47
zygamvo: hold on13:47
mvomborzecki: indeed13:48
zygamvo: note that I'm measuring inside the process13:49
zygabut now I see 0ms on trivial apps13:49
zygalet me try brave13:49
zyga0ms rounded to ms13:49
zygain snap-confine I will measure the whole chunk13:49
zygaho issues14:01
zygaI seem to be stuck at the joining screen14:01
zyga2fa14:02
mborzeckimvo: perf trace https://paste.ubuntu.com/p/kpNMpxj5jm/14:37
mvomborzecki: thanks!14:38
mborzeckimvo: mksquashfs options -noI -noD -noF14:47
mvomborzecki: thank you14:48
mborzeckimvo: mount thing reproduces on 18.04  too14:56
* cachio lunch14:57
mborzeckimvo: what do you think about cherry-picking centos support to 2.36?14:58
mvomborzecki: +114:59
mvomborzecki: how big is the diff?14:59
mupPR snapd#6173 closed: sanity: extend the kernel version check to cover CentOS/RHEL kernels <Created by bboozzoo> <Merged by bboozzoo> <https://github.com/snapcore/snapd/pull/6173>14:59
mborzeckimvo: most of it are tests, a little of packaging, i'd pick the sanity check too15:00
mborzeckimvo: i can open a PR15:00
mvomborzecki: please do15:00
mborzeckimvo: running under spread now15:08
zygamvo, mborzecki: I need to land https://github.com/snapcore/snapd/pull/617015:24
mupPR #6170: overlord,tests: expose enabled features to /var/lib/snapd/features <Per-user mount ns  🐎> <Created by zyga> <https://github.com/snapcore/snapd/pull/6170>15:24
zygamvo: I'm preparing to send feature PRs15:25
mvopedronis: I guess 6170 is uncontroversial, right? a look at the description is enough, we can do the in-depth review15:27
zygamvo: then I need this https://github.com/snapcore/snapd/pull/618115:48
mupPR snapd#6181 opened: features: add feature test methods <Per-user mount ns  🐎> <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6181>15:48
mupPR #6181: features: add feature test methods <Per-user mount ns  🐎> <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6181>15:48
mupPR snapd#6182 opened: Revert "cmd/snap-confine: don't allow mapping lib{uuid,blkid}" <Created by zyga> <https://github.com/snapcore/snapd/pull/6182>15:51
mupPR snapd#6183 opened: sanity, spread, tests: add CentOS (2.36) <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6183>15:55
mborzeckimvo: ^^15:55
mvomborzecki: ta15:59
zygamvo: prepping remaining branches16:06
mvozyga: thank you!16:08
zygaadding missing tests etc16:09
pedronismvo: yes, about 6170 seems fine, we could bike shed about the name features, but I know other cases where it has been used fairly generically16:14
mupPR snapcraft#2413 opened: kernel plugin: introduce a _get_kernel_version() helper <Created by piso77> <https://github.com/snapcore/snapcraft/pull/2413>16:18
mvopedronis: great, thanks. yeah, lets not bike shed too much :)16:18
pedronismvo: what is #6178 about, startup performance of snaps?16:21
mupPR #6178: snap,snap-exec: add SNAP_DEBUG_START_TIME environment <Created by mvo5> <https://github.com/snapcore/snapd/pull/6178>16:22
mvopedronis: yeah, the brave discussion on the mailinglist16:24
* roadmr prefers cowardly discussion :P16:24
mvopedronis: happy to give you more context if you want, we can have a quick HO or something (or tomorrow or later, whatever works for you)16:24
zygamvo: https://github.com/snapcore/snapd/pull/618416:26
mupPR #6184: perf: add performance helpers <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6184>16:26
mupPR snapd#6184 opened: perf: add performance helpers <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6184>16:26
zygamvo: the next part is the perf manager but I need to make tea now16:27
zygaI'm starting to feel sick16:27
mvozyga: yeah, lets get pedronis a chance to look at this too16:27
mvozyga: uh, get well!16:27
mvozyga: don't get sick - we need you :)16:27
zygamvo: sure, I will open all the PRs so we can see the ful set16:27
mvook16:28
zygamvo: I hope to be okay but I think this autumn morning was not my best16:28
pedronismvo: that's ok, but if perf of startup is a focus, can we have a card ?16:34
zygapedronis: I made a card recently16:35
pedroniszyga: mmh, it's only for you,  mvo is not on it16:35
mvopedronis: I think they are slightly different16:36
mvopedronis: and will converge16:36
zygayes16:36
pedroniszyga: mvo: anyway if it really involves adding a new manager, yes I should be in discussion16:36
zygamvo's work will improve perf startu16:36
mvopedronis: zyga was working on internal perfoamnce, e.g. seccomp setup16:36
zygamy work will allow us to mearure it better16:36
mvopedronis: I am working on snap run -> app overhead16:36
pedronismvo: yes, so please make a different card16:36
mvopedronis: will do16:36
pedronisthx16:37
pedronismvo: I imagine it will take a bit of time, not just a quick thing16:37
mvopedronis: yeah, eating all day so far but good results16:37
pedronismvo: :)16:38
pedroniszyga: it's a bit unclear to me how #6184 would interact with things that restart snapd16:41
mupPR #6184: perf: add performance helpers <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6184>16:41
zygapedronis: the perf manager can save its state if the perf monitoring feature is enabled16:41
zygapedronis: perf manager is in the next branch, just adding missing tests and making sure it's all tidy16:42
zygapedronis: where state is just the set of samples16:42
zygapedronis: this is not the regular state16:42
zygapedronis: it is not persisted in normal cases16:43
pedroniszyga: how do you make it a noop? or is the plan to use it only when needed?16:43
zygapedronis: you have to enable it16:43
zygapedronis: there's a feature flag16:43
pedroniszyga: anyway sounds we need to discuss it more16:43
pedroniszyga: a feature flag?16:43
pedroniswhere?16:43
zygapedronis: it's all coming up in subsequent branches I'm chopping from what I implemnted today16:43
pedronisa config ?16:43
zygapedronis: if you set a core config experimental.performance-measurements the manager stops being a no-op16:44
pedroniszyga: we didn't discuss the plan, did you discuss it with mvo?16:44
zygaotherwise the ring bufer is set to nil and nothing happens16:44
zygapedronis: just during the call today, all of this code was made today16:44
pedroniszyga: thagt's great, but it wil take a while to revirw16:44
zygapedronis: the short version is that perf manager doesn't do anything unless enabled16:44
pedroniszyga: it's a bit unclear where it fits overall16:45
pedronisas well16:45
zygapedronis: not sure what you mean16:45
pedroniszyga: is it about first boot perf? about something else?16:45
zygapedronis: the idea was to store samples in memory in snapd and expose it for debugging16:45
pedroniszyga: you understand that the code need not to crash normal snapd16:45
pedronisusage16:45
zygapedronis: about figuring out what is slow in couple of things we are experiencing16:45
zygayes16:46
zygain normal code it's a nop16:46
zygas/normal code/normal mode when feature is not active/16:46
pedroniszyga: just saying that the fact that it was quick to write, doesn't mean it can be quick to land16:46
zygayes, I understand that16:46
zygaI don't even have to land it, really it was made to debug ongoing issues16:46
zygawe can land it slowly16:46
zygabut we can use the collective branch for experiments16:47
zygae.g. we now know that seccomp setup is slower than apparmor16:47
pedronisok16:47
zygaand we have precise timing data on snap-update-ns16:47
zyga(we can collect snap-{run,confine,exec} timing this way too16:47
zygaif the manager is enabled all that happens is we store samples in a ring buffer16:48
pedroniszyga: the code is go, no?16:48
zygapedronis: yes16:48
pedronishow does it relate to snap-confine?16:48
pedronisyou need a c variant?16:48
zygapedronis: processes that are not in snapd can also collect samples16:48
zygapedronis: really just start/end/summary16:48
zygapedronis: snapd collects that16:48
zygapedronis: and exposes everything via "snap debug speed"16:48
zygapedronis: again, only if the feature is enabled16:49
zygapedronis: such samples are saved in /runs/snapd/perf/*.json and are loaded by the perf manager16:49
zygapedronis: either in ensure or when "snap debug speed" is used16:49
pedroniszyga: that sounds delicate16:50
zygapedronis: that last command just shows collected samples and shows us data16:50
pedronisbecause Ensure is in the hot path as well16:50
zygaer, duration and meta-data16:50
zygait doesn't really need to be in ensure because it's collected on demand in the API debug call16:50
pedronisexcept if you restart16:50
zygaif the manager is stopped is saves the samples to /run/snapd/perf/16:51
zygaso on restart it can pick it up16:51
zygaI haven't used that much though because for local testing with several snaps it wasn't the interesting case16:51
pedroniszyga: I marked it as blocked, not because it not useful, I'm just worried about landing it in prod snapd for a while16:51
zygapedronis: sure16:52
zygapedronis: I will propose all the branches and collect them all in a evaluation branch for ongoing debugging16:52
pedroniszyga: what's the state of per-user mounts?16:53
zygapedronis: under review, one branch needs my activity but master was wonky so I didn't push it forward yet16:54
pedronisok16:54
zygapedronis: I will fix that branch because the issue is small, just sequencing of other PRs16:54
zygapedronis: I really want to end that work :)16:54
* zyga -> break for tea16:56
mupPR snapd#6185 opened: snap: add new `snap run --perf` call <Performance 🚀> <Created by mvo5> <https://github.com/snapcore/snapd/pull/6185>17:30
=== pstolowski is now known as pstolowski|afk
zygamvo: hey18:25
zygaif you are still working I would love to review and land https://github.com/snapcore/snapd/pull/618118:25
mupPR #6181: features: add feature test methods <Per-user mount ns  🐎> <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6181>18:25
zygaas well as https://github.com/snapcore/snapd/pull/617018:26
mupPR #6170: overlord,tests: expose enabled features to /var/lib/snapd/features <Per-user mount ns  🐎> <Created by zyga> <https://github.com/snapcore/snapd/pull/6170>18:26
zygaI'm blocked by those two18:26
mupPR snapcraft#2414 opened: yaml_utils: allow unicode while encoding <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2414>18:42
zygajdstrand: can you please look at 618219:40
ijohnsonzyga: I think jdstrand is out today19:40
zygabummer19:41
mupPR snapd#6105 closed: NOT REVIEW: New task to force error on degraded test <Created by sergiocazzolato> <Closed by sergiocazzolato> <https://github.com/snapcore/snapd/pull/6105>19:55
mupPR snapd#6148 closed: cmd/snap-confine: remove unneeded unshare <Per-user mount ns  🐎> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/6148>23:00

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