mup | PR 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 |
---|---|---|
mvo | zyga: hey, 5845 is ready for a re-review (you commented some days ago there) | 06:17 |
mborzecki | morning | 06:19 |
mvo | hey mborzecki | 06:21 |
mborzecki | mvo: how are the tests looking today? still mostly red? | 06:21 |
mvo | mborzecki: I saw a green one | 06:22 |
mvo | mborzecki: 5845 which also needs a second review | 06:22 |
mvo | mborzecki: but also some reds :( | 06:22 |
mvo | mborzecki: I just retriggered 6156, that should tell us about the testing weather today | 06:22 |
mborzecki | heh :) | 06:22 |
zyga | o/ | 06:56 |
zyga | mvo: looking | 06:56 |
zyga | 1K wow | 06:57 |
mvo | zyga: 1k? | 07:00 |
zyga | about 1K of diff | 07:00 |
zyga | but no worries, reading | 07:01 |
mup | PR snapd#6178 opened: snap,snap-exec: add SNAP_DEBUG_START_TIME environment <Created by mvo5> <https://github.com/snapcore/snapd/pull/6178> | 07:11 |
mvo | zyga: heh, things look pretty good with snap_debug_start_time set, it seems we take only ~0.1sec for brave to setup our stuff | 07:24 |
zyga | nice | 07:24 |
zyga | so what is slow? | 07:24 |
mvo | zyga: I think the wrappers, I'm on it, not sure conclusively yet | 07:25 |
zyga | super | 07:25 |
mvo | zyga: slowly working my way up :) | 07:25 |
zyga | I will finish this review, garden my branches and work more on perf monitoring | 07:25 |
zyga | while not immediately helpful to your cause it will allow us to do this more or less automatically down the line | 07:26 |
mborzecki | mvo: wonder if something like snap run --strace='--raw -e execve -r' <snap.app> would work | 07:31 |
mvo | mborzecki: hm, good idea, maybe no need for a special tool. | 07:32 |
mborzecki | looks a bit noisy for gnome-calculator | 07:32 |
mvo | mborzecki: heh, its even worse with brave :) | 07:32 |
mvo | mborzecki: forkstat is what I'm currently using | 07:32 |
mborzecki | mvo: well, with gnome calc, there's clearly lots of stuff happening | 07:34 |
mvo | mborzecki: yeah, tons | 07:34 |
mvo | mborzecki: including update-mime-database which is ~2s here | 07:34 |
mborzecki | mvo: gnome-calcualtor https://paste.fedoraproject.org/paste/rCbAYtyL02YLJ9xm~-pTZg/raw | 07:36 |
mborzecki | heh, funny how not using absolute path makes multiple execve() calls | 07:37 |
zyga | mvo: reviewed | 07:39 |
mborzecki | hm xdg-user-dirs update takes less time than head -c0 calls | 07:39 |
mvo | zyga: ta | 07:40 |
zyga | mvo: I'm sorry that it is not a +1 yet | 07:40 |
zyga | let me know if you want to talk about any of this | 07:41 |
mvo | zyga: no worries, thanks for the review, the comments look really useful | 07:47 |
mborzecki | mvo: 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 total | 07:50 |
mborzecki | mvo: that's for gnome-calculator | 07:50 |
zyga | mborzecki: that's all in shell, right? | 07:51 |
mborzecki | mvo: yes | 07:52 |
zyga | a shell program making those calls? | 07:52 |
mvo | mborzecki: whats the total overhead for you? | 07:52 |
mborzecki | mvo: ~2s | 07:54 |
mborzecki | mvo: but that's ssd, multi core and so on :P | 07:54 |
mborzecki | mvo: i'll try with dropped caches | 07:55 |
=== pstolowski|afk is now known as pstolowski | ||
pstolowski | mornings | 07:58 |
zyga | (mac chime) | 08:00 |
zyga | hey pawel | 08:00 |
pstolowski | :) | 08:00 |
pstolowski | can 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 |
mvo | mborzecki: ta, I see ~2s here as well (also multi-core, ssd etc) | 08:04 |
mvo | pstolowski: good morning! | 08:05 |
zyga | yep, checking | 08:05 |
mborzecki | mvo: 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 minimal | 08:14 |
zyga | So what is the slow part? | 08:15 |
mborzecki | mvo: https://paste.fedoraproject.org/paste/w-X6E2U7OKjM9n0210BGTA see line 644, there's a bunch of processes started, but they don't exec | 08:15 |
zyga | Apart from fonts | 08:15 |
mborzecki | mvo: some glib/gtk thing? | 08:15 |
zyga | To compute 2+2 you need to fork 100 times | 08:17 |
mborzecki | g_async_* | 08:19 |
mborzecki | maybe is spawning gjs under the hood :) | 08:19 |
mvo | mborzecki: looking | 08:21 |
mvo | mborzecki: why does a pastebin require 7 script to work - oh well | 08:21 |
mvo | mborzecki: hard to say but I would not be surprised if that was some sort of gjs | 08:21 |
pstolowski | zyga: any findings? | 08:34 |
pstolowski | mborzecki: you landed some improvements for diffing on failing go tests some time ago; what's the approximate location of this diffing code? | 08:41 |
mborzecki | pstolowski: that'd be in check.v1 | 08:41 |
mborzecki | pstolowski: checkers.go, there's formatUnequal() | 08:42 |
pstolowski | mborzecki: got it, thanks! | 08:43 |
pstolowski | mborzecki: heh, it seems it's related to the leak i'm seeing | 08:44 |
pstolowski | mborzecki: i removed that entire logic | 08:44 |
pstolowski | mborzecki: getting just a few test failures now (expected) | 08:44 |
mborzecki | pstolowski: is that in interfaces? | 08:44 |
mborzecki | i recall getting oom when something went awry there | 08:44 |
pstolowski | it in snap/ , yaml parsing tests | 08:45 |
pstolowski | mborzecki: it eats mem pretty quickly with the diff i pasted earlier | 08:45 |
mborzecki | pstolowski: iirc it didn't like when structs were pointing to each other | 08:45 |
pstolowski | mborzecki: 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 |
pstolowski | mborzecki: in other words i deliberately broke code not to create references, and it causes oom :/ | 08:47 |
mborzecki | zyga: mvo: i think g-c is actually pulling some exchange rate data from the network | 08:47 |
mvo | mborzecki: I wrote a small script to gather the strace datahttps://paste.ubuntu.com/p/vD2rsyXHJr/ | 08:48 |
mvo | 08:48 | |
mvo | mborzecki: https://paste.ubuntu.com/p/vD2rsyXHJr/ | 08:48 |
mvo | mborzecki: this is from brave, I will post the script in a sec | 08:48 |
pstolowski | mborzecki: i suppose the problem is actually in pretty? | 08:48 |
mborzecki | mvo: nice | 08:48 |
mborzecki | pstolowski: yes, that's quite possible | 08:49 |
zyga | pstolowski: re, sorry for the lag - some house stuff | 08:57 |
zyga | pstolowski: looking again | 08:58 |
pstolowski | np | 08:58 |
zyga | pstolowski: ah so check.v1 bug? | 08:58 |
pstolowski | zyga: it seems so | 08:58 |
pstolowski | zyga: or rather - it's dependency that's doing the diffing | 08:59 |
mborzecki | pstolowski: keep in mind that the print only happens if the values are not equal in the tests | 09:02 |
pstolowski | mborzecki: sure | 09:03 |
pstolowski | mborzecki: only it's impossible to find out what's failing ;) | 09:03 |
pstolowski | mborzecki: for now i just disabled this pretty printing | 09:04 |
mborzecki | pstolowski: i can relate :) this was super annoying when working on interfaces | 09:04 |
mup | PR snapd#6172 closed: Send epochs followup <Created by chipaca> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/6172> | 09:05 |
mborzecki | mvo: 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 |
mborzecki | suprising how far one can go solely with emacs :) | 09:17 |
zyga | mborzecki: I think I should measure snap-update-ns | 09:17 |
zyga | can you please tell me what is the mount profile there | 09:17 |
zyga | both system and user | 09:17 |
mvo | mborzecki: aha, ok | 09:18 |
mvo | mborzecki: http://paste.ubuntu.com/p/Gn95x59tqT/ <- this is what I used | 09:19 |
mborzecki | zyga: http://paste.ubuntu.com/p/tg5VcSyJw8/ | 09:19 |
mvo | mborzecki: I will include it in my bench script to make reproducing this easier | 09:19 |
mborzecki | mvo: great | 09:20 |
zyga | thanks | 09:25 |
zyga | mborzecki: is that all? | 09:26 |
zyga | that's the system profile, is there a user profile too? | 09:26 |
zyga | maybe there's a bug but I don't see why this should take 700ms | 09:27 |
zyga | well | 09:27 |
zyga | something to check | 09:27 |
mborzecki | zyga: https://paste.ubuntu.com/p/wG63T4jQjd/ | 09:27 |
zyga | ah | 09:27 |
zyga | so yes | 09:27 |
zyga | portals | 09:27 |
zyga | I bet a beer it is portal startup check | 09:27 |
zyga | though | 09:27 |
* zyga thinks | 09:27 | |
zyga | perhaps not | 09:27 |
zyga | that was added to snap-run IIRC | 09:28 |
zyga | mborzecki: when you measured, 700 was for which of the two executions of snap-update-ns? | 09:28 |
mborzecki | zyga: regular one, let me grab that part of strace | 09:29 |
zyga | with roughly 30 mount calls there that's ~~ 20 ms per entry | 09:30 |
zyga | (aka looooooots) | 09:31 |
mborzecki | zyga: https://paste.ubuntu.com/p/mR7vpYFRgS/ | 09:33 |
mborzecki | zyga: hm, it'd be easier if snap-update-ns printed it's pid :) | 09:34 |
zyga | is that [pid 20700] 0.674152 [????????????????] +++ exited with 0 +++ | 09:35 |
zyga | 0.67 is the duration? | 09:35 |
mborzecki | zyga: yes, that's my guess | 09:36 |
mvo | mborzecki: I started using "-ttt" to get more reliable timing | 09:39 |
mup | PR snapd#6179 opened: snap: epoch lists must contain no duplicate entries <Created by chipaca> <https://github.com/snapcore/snapd/pull/6179> | 09:39 |
mborzecki | mvo: yeah, seems to give quite different results when tracking many syscalls | 09:47 |
mborzecki | zyga: 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 |
zyga | what is -ttt? | 09:49 |
zyga | given that this x7 difference is the remaining allocation accurate? | 09:50 |
zyga | (to other programs in the chain) | 09:50 |
kjackal | Hi 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/edge | 09:51 |
mborzecki | zyga: -t is wall clock, -r is relative time between syscalls (supposedly using montonic clock) | 09:51 |
mborzecki | zyga: also, from the point where sc_fork_helper is called to where sigchld is received it's ~650ms | 09:52 |
zyga | that's ok | 09:53 |
zyga | sc_fork_helper lives for the duration of both s-u-n calls | 09:53 |
zyga | in between we do our business and call sun twice | 09:53 |
mborzecki | hm actually intersting, between calling sc_fork_helper and sc_populate_mount_ns there's 500ms difference, not that it matters much | 09:55 |
zyga | mmmmmmm | 09:55 |
zyga | that's interesting | 09:55 |
zyga | note | 09:55 |
zyga | which version is this | 09:55 |
zyga | that code changed recently | 09:55 |
zyga | is this master? | 09:55 |
mborzecki | i've built s-u-n from master btw | 09:56 |
zyga | ah sorry | 09:56 |
zyga | master is "old" | 09:56 |
zyga | I was thinking about my recent changes | 09:56 |
zyga | so | 09:56 |
zyga | hmmm | 09:56 |
zyga | maybe the slow part is aa change hat? | 09:57 |
mborzecki | zyga: lines 8 and 15 in that last paste | 09:57 |
zyga | can you do an experiment | 09:57 |
zyga | comment that out | 09:57 |
zyga | and see what happens | 09:57 |
zyga | you may need to splice the helper hat profile into the main profile to avoid denials | 09:57 |
zyga | it'd be fun if that small operation took this long | 09:57 |
* zyga is happy snap-confine is in C an can be timed reliably with symbols | 09:58 | |
mup | PR 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_ | ||
zyga | brb | 10:33 |
mup | PR 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 |
mup | PR 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 lunch | 11:21 | |
mborzecki | mvo: 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 mount | 11:47 |
mborzecki | mvo: 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 disappear | 11:48 |
mborzecki | mvo: so the only path where it fails reliably is when reload interleaves with start (and maybe stop) | 11:49 |
mborzecki | Chipaca: have you seen https://forum.snapcraft.io/t/display-non-autoconnect-interfaces-at-install/8609 ? sounds like a nice ux improvement | 11:52 |
mvo | mborzecki: nice findings"! | 11:57 |
mvo | mborzecki: this also means we can serialize daemon reload and the problem hopefully goes away? | 11:57 |
mvo | GRL (GLOBAL RELOAD LOCK) | 11:57 |
mup | PR snapcraft#2412 opened: schema: add support for title <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2412> | 12:14 |
cachio | zyga, https://paste.ubuntu.com/p/j7BdJ6jhJJ/ | 12:21 |
cachio | any idea what could cause that? | 12:21 |
cachio | it is using core from edge | 12:21 |
cachio | mvo, ~ | 12:21 |
zyga | yes | 12:21 |
zyga | but no idea why | 12:21 |
zyga | what distribution and kernel is this? | 12:22 |
cachio | bionic | 12:22 |
cachio | 4.15.0-36-generic | 12:22 |
zyga | mborzecki: ^ | 12:22 |
zyga | is the calc open now? | 12:22 |
mborzecki | hah, intersting | 12:23 |
mborzecki | zyga: does edge have the fixes you landed recently? | 12:23 |
zyga | mborzecki: yes | 12:23 |
cachio | no | 12:23 |
zyga | well | 12:23 |
zyga | I suspect so | 12:23 |
mborzecki | yes/no/maybe :) | 12:23 |
zyga | but in either case those are not related | 12:24 |
zyga | none affect discarding | 12:24 |
mborzecki | maybe gnome-calculator is still working? | 12:24 |
cachio | it is not working now | 12:25 |
cachio | no process running at least | 12:25 |
zyga | can you look at | 12:26 |
zyga | cd /sys/fs/cgroup/freezer | 12:26 |
cachio | I tried another snap | 12:26 |
cachio | supercalc-snap 1 stable keshavnrj disabled,broken | 12:26 |
cachio | and also failed | 12:26 |
zyga | then cd to snap.gnome-calculator | 12:26 |
zyga | and look at cgroup.procs | 12:26 |
zyga | check what processes are there | 12:26 |
cachio | zyga, I dont see calculator | 12:28 |
zyga | what do you see | 12:28 |
cachio | zyga, other gnome snapd | 12:28 |
cachio | snaps | 12:28 |
zyga | can you run | 12:29 |
cachio | logs, characters | 12:29 |
zyga | sudo SNAPD_DEBUG=1 /usr/lib/snapd/snap-discard-ns gnome-calculator | 12:29 |
cachio | zyga, what I see | 12:30 |
cachio | is that if I remove from command line a snap | 12:30 |
cachio | it works | 12:30 |
cachio | if I remove from ubuntu-software the snap fails to be removed | 12:30 |
cachio | zyga, https://paste.ubuntu.com/p/cNHpvdCqMM/ | 12:31 |
zyga | that's the one from the distro, can you run the one from core | 12:32 |
zyga | that will be representative of what snapd runs | 12:32 |
zyga | it has more stuff going on | 12:32 |
zyga | sorry, I didn't think about this | 12:33 |
cachio | zyga, I already tried removing from core and it works | 12:33 |
zyga | ? | 12:33 |
zyga | I mean | 12:33 |
zyga | can you try running snap-discard-ns like you did but use /snap/core/current/usr/lib/snapd/snap-discard-ns instead | 12:33 |
cachio | yes | 12:34 |
zyga | and capturing the log if that | 12:34 |
zyga | when it fails | 12:34 |
cachio | https://paste.ubuntu.com/p/9Ng8pfysj9/ | 12:35 |
cachio | fails | 12:35 |
zyga | aha | 12:36 |
zyga | I suspect I know what the problem is | 12:36 |
zyga | this is fixed (I suspect) by ... | 12:36 |
zyga | https://github.com/snapcore/snapd/pull/6159 | 12:36 |
mup | PR #6159: cmd/snap-confine: handle mounted shared /run/snapd/ns <Created by zyga> <https://github.com/snapcore/snapd/pull/6159> | 12:37 |
zyga | cachio: can you post your host's mountinfo table | 12:37 |
zyga | what I don't know is how that can happen | 12:37 |
zyga | seems like another bug | 12:37 |
Chipaca | mborzecki: yes i saw that | 12:37 |
Chipaca | mborzecki: and yes i agree | 12:37 |
mborzecki | Chipaca: we should have a papercuts lane in trello | 12:37 |
Chipaca | mborzecki: my plate is full but i've got my eyes on it in case nobody picks it up before i finish the smallest pea | 12:37 |
Chipaca | mborzecki: +23 | 12:38 |
zyga | note how we never unmounted the file | 12:38 |
* Chipaca accidentally burned through all his +1 reserves | 12:38 | |
zyga | just unliked it | 12:38 |
zyga | that is because it's a tmpfs | 12:38 |
zyga | because we re-mounted /run/snapd/ns over itself | 12:38 |
zyga | hiding mounts inside | 12:38 |
zyga | not sure why | 12:38 |
cachio | zyga, https://paste.ubuntu.com/p/H5sCQkVWkm/ | 12:38 |
zyga | can you get the mountinfo table instead | 12:38 |
zyga | mounts shows partial data | 12:39 |
zyga | but yeah, I suspect that's the case | 12:40 |
zyga | tmpfs /run/snapd/ns tmpfs rw,nosuid,noexec,relatime,size=1222540k,mode=755 0 0 | 12:40 |
zyga | tmpfs /run/snapd/ns tmpfs rw,nosuid,noexec,relatime,size=1222540k,mode=755 0 0 | 12:40 |
cachio | zyga, sorry, this is what you need https://paste.ubuntu.com/p/YrDVdcNyQV/? | 12:44 |
cachio | zyga, sorry, this is what you need https://paste.ubuntu.com/p/YrDVdcNyQV/ ? | 12:44 |
zyga | no | 12:45 |
zyga | please cat /proc/self/mountinfo | 12:45 |
zyga | brb, lunch | 12:45 |
zyga | (not urgent) | 12:45 |
zyga | cachio: I think the bug is fixed in that PR I linked to | 12:45 |
cachio | zyga, https://paste.ubuntu.com/p/Fhb8GrpJnM/ | 12:45 |
zyga | thanks, I'll check soon] | 12:46 |
cachio | zyga, ok, but why if I do snap remove foo | 12:46 |
cachio | works | 12:46 |
cachio | and if I remove from ubuntu software it fails? | 12:46 |
Chipaca | mborzecki: trello says it isn't for papercut kind of thing | 12:49 |
Chipaca | mborzecki: so, https://forum.snapcraft.io/tags/papercut | 12:49 |
Chipaca | mborzecki: (in "about the snapd team board", no backlog and no quick tasks) | 12:50 |
mborzecki | Chipaca: ah, fair enough | 12:54 |
mborzecki | zyga: https://github.com/systemd/systemd/issues/10872 | 12:55 |
zyga | mborzecki: nice | 13:07 |
zyga | mborzecki: I'm not 100% sure it's systemd but let's see | 13:07 |
mborzecki | zyga: couldn't reproduce it any other way | 13:07 |
zyga | cachio: it depends on the order in the info table | 13:07 |
zyga | mborzecki: I will look again later | 13:07 |
zyga | mborzecki: need to wrap up and wrap up and do more tasks | 13:07 |
cachio | zyga, ahh, ok | 13:09 |
zyga | mvo: I now have collection of performance events from snap-update-ns | 13:29 |
zyga | detailed actions as well | 13:29 |
zyga | now looking at C | 13:29 |
mup | PR # closed: core-build#11, core-build#22, core-build#26, core-build#37 | 13:30 |
mup | PR # opened: core-build#11, core-build#22, core-build#26, core-build#37 | 13:31 |
* Chipaca hugs mup | 13:32 | |
Chipaca | some days are like that | 13:32 |
mvo | mborzecki: super nice | 13:44 |
mvo | zyga: cool, thanks. what times are we talking about? | 13:44 |
mvo | zyga: I mean, how much time does snap-update-ns take on a typical snap? | 13:44 |
mborzecki | mvo: zyga: it'd be nice to have a baseline reference, eg. hello-world and then the snap with all the interfaces | 13:47 |
zyga | nice idea | 13:47 |
zyga | mvo: hold on | 13:47 |
mvo | mborzecki: indeed | 13:48 |
zyga | mvo: note that I'm measuring inside the process | 13:49 |
zyga | but now I see 0ms on trivial apps | 13:49 |
zyga | let me try brave | 13:49 |
zyga | 0ms rounded to ms | 13:49 |
zyga | in snap-confine I will measure the whole chunk | 13:49 |
zyga | ho issues | 14:01 |
zyga | I seem to be stuck at the joining screen | 14:01 |
zyga | 2fa | 14:02 |
mborzecki | mvo: perf trace https://paste.ubuntu.com/p/kpNMpxj5jm/ | 14:37 |
mvo | mborzecki: thanks! | 14:38 |
mborzecki | mvo: mksquashfs options -noI -noD -noF | 14:47 |
mvo | mborzecki: thank you | 14:48 |
mborzecki | mvo: mount thing reproduces on 18.04 too | 14:56 |
* cachio lunch | 14:57 | |
mborzecki | mvo: what do you think about cherry-picking centos support to 2.36? | 14:58 |
mvo | mborzecki: +1 | 14:59 |
mvo | mborzecki: how big is the diff? | 14:59 |
mup | PR 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 |
mborzecki | mvo: most of it are tests, a little of packaging, i'd pick the sanity check too | 15:00 |
mborzecki | mvo: i can open a PR | 15:00 |
mvo | mborzecki: please do | 15:00 |
mborzecki | mvo: running under spread now | 15:08 |
zyga | mvo, mborzecki: I need to land https://github.com/snapcore/snapd/pull/6170 | 15:24 |
mup | PR #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 |
zyga | mvo: I'm preparing to send feature PRs | 15:25 |
mvo | pedronis: I guess 6170 is uncontroversial, right? a look at the description is enough, we can do the in-depth review | 15:27 |
zyga | mvo: then I need this https://github.com/snapcore/snapd/pull/6181 | 15:48 |
mup | PR 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 |
mup | PR #6181: features: add feature test methods <Per-user mount ns 🐎> <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6181> | 15:48 |
mup | PR 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 |
mup | PR snapd#6183 opened: sanity, spread, tests: add CentOS (2.36) <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/6183> | 15:55 |
mborzecki | mvo: ^^ | 15:55 |
mvo | mborzecki: ta | 15:59 |
zyga | mvo: prepping remaining branches | 16:06 |
mvo | zyga: thank you! | 16:08 |
zyga | adding missing tests etc | 16:09 |
pedronis | mvo: yes, about 6170 seems fine, we could bike shed about the name features, but I know other cases where it has been used fairly generically | 16:14 |
mup | PR snapcraft#2413 opened: kernel plugin: introduce a _get_kernel_version() helper <Created by piso77> <https://github.com/snapcore/snapcraft/pull/2413> | 16:18 |
mvo | pedronis: great, thanks. yeah, lets not bike shed too much :) | 16:18 |
pedronis | mvo: what is #6178 about, startup performance of snaps? | 16:21 |
mup | PR #6178: snap,snap-exec: add SNAP_DEBUG_START_TIME environment <Created by mvo5> <https://github.com/snapcore/snapd/pull/6178> | 16:22 |
mvo | pedronis: yeah, the brave discussion on the mailinglist | 16:24 |
* roadmr prefers cowardly discussion :P | 16:24 | |
mvo | pedronis: 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 |
zyga | mvo: https://github.com/snapcore/snapd/pull/6184 | 16:26 |
mup | PR #6184: perf: add performance helpers <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6184> | 16:26 |
mup | PR snapd#6184 opened: perf: add performance helpers <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6184> | 16:26 |
zyga | mvo: the next part is the perf manager but I need to make tea now | 16:27 |
zyga | I'm starting to feel sick | 16:27 |
mvo | zyga: yeah, lets get pedronis a chance to look at this too | 16:27 |
mvo | zyga: uh, get well! | 16:27 |
mvo | zyga: don't get sick - we need you :) | 16:27 |
zyga | mvo: sure, I will open all the PRs so we can see the ful set | 16:27 |
mvo | ok | 16:28 |
zyga | mvo: I hope to be okay but I think this autumn morning was not my best | 16:28 |
pedronis | mvo: that's ok, but if perf of startup is a focus, can we have a card ? | 16:34 |
zyga | pedronis: I made a card recently | 16:35 |
pedronis | zyga: mmh, it's only for you, mvo is not on it | 16:35 |
mvo | pedronis: I think they are slightly different | 16:36 |
mvo | pedronis: and will converge | 16:36 |
zyga | yes | 16:36 |
pedronis | zyga: mvo: anyway if it really involves adding a new manager, yes I should be in discussion | 16:36 |
zyga | mvo's work will improve perf startu | 16:36 |
mvo | pedronis: zyga was working on internal perfoamnce, e.g. seccomp setup | 16:36 |
zyga | my work will allow us to mearure it better | 16:36 |
mvo | pedronis: I am working on snap run -> app overhead | 16:36 |
pedronis | mvo: yes, so please make a different card | 16:36 |
mvo | pedronis: will do | 16:36 |
pedronis | thx | 16:37 |
pedronis | mvo: I imagine it will take a bit of time, not just a quick thing | 16:37 |
mvo | pedronis: yeah, eating all day so far but good results | 16:37 |
pedronis | mvo: :) | 16:38 |
pedronis | zyga: it's a bit unclear to me how #6184 would interact with things that restart snapd | 16:41 |
mup | PR #6184: perf: add performance helpers <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6184> | 16:41 |
zyga | pedronis: the perf manager can save its state if the perf monitoring feature is enabled | 16:41 |
zyga | pedronis: perf manager is in the next branch, just adding missing tests and making sure it's all tidy | 16:42 |
zyga | pedronis: where state is just the set of samples | 16:42 |
zyga | pedronis: this is not the regular state | 16:42 |
zyga | pedronis: it is not persisted in normal cases | 16:43 |
pedronis | zyga: how do you make it a noop? or is the plan to use it only when needed? | 16:43 |
zyga | pedronis: you have to enable it | 16:43 |
zyga | pedronis: there's a feature flag | 16:43 |
pedronis | zyga: anyway sounds we need to discuss it more | 16:43 |
pedronis | zyga: a feature flag? | 16:43 |
pedronis | where? | 16:43 |
zyga | pedronis: it's all coming up in subsequent branches I'm chopping from what I implemnted today | 16:43 |
pedronis | a config ? | 16:43 |
zyga | pedronis: if you set a core config experimental.performance-measurements the manager stops being a no-op | 16:44 |
pedronis | zyga: we didn't discuss the plan, did you discuss it with mvo? | 16:44 |
zyga | otherwise the ring bufer is set to nil and nothing happens | 16:44 |
zyga | pedronis: just during the call today, all of this code was made today | 16:44 |
pedronis | zyga: thagt's great, but it wil take a while to revirw | 16:44 |
zyga | pedronis: the short version is that perf manager doesn't do anything unless enabled | 16:44 |
pedronis | zyga: it's a bit unclear where it fits overall | 16:45 |
pedronis | as well | 16:45 |
zyga | pedronis: not sure what you mean | 16:45 |
pedronis | zyga: is it about first boot perf? about something else? | 16:45 |
zyga | pedronis: the idea was to store samples in memory in snapd and expose it for debugging | 16:45 |
pedronis | zyga: you understand that the code need not to crash normal snapd | 16:45 |
pedronis | usage | 16:45 |
zyga | pedronis: about figuring out what is slow in couple of things we are experiencing | 16:45 |
zyga | yes | 16:46 |
zyga | in normal code it's a nop | 16:46 |
zyga | s/normal code/normal mode when feature is not active/ | 16:46 |
pedronis | zyga: just saying that the fact that it was quick to write, doesn't mean it can be quick to land | 16:46 |
zyga | yes, I understand that | 16:46 |
zyga | I don't even have to land it, really it was made to debug ongoing issues | 16:46 |
zyga | we can land it slowly | 16:46 |
zyga | but we can use the collective branch for experiments | 16:47 |
zyga | e.g. we now know that seccomp setup is slower than apparmor | 16:47 |
pedronis | ok | 16:47 |
zyga | and we have precise timing data on snap-update-ns | 16:47 |
zyga | (we can collect snap-{run,confine,exec} timing this way too | 16:47 |
zyga | if the manager is enabled all that happens is we store samples in a ring buffer | 16:48 |
pedronis | zyga: the code is go, no? | 16:48 |
zyga | pedronis: yes | 16:48 |
pedronis | how does it relate to snap-confine? | 16:48 |
pedronis | you need a c variant? | 16:48 |
zyga | pedronis: processes that are not in snapd can also collect samples | 16:48 |
zyga | pedronis: really just start/end/summary | 16:48 |
zyga | pedronis: snapd collects that | 16:48 |
zyga | pedronis: and exposes everything via "snap debug speed" | 16:48 |
zyga | pedronis: again, only if the feature is enabled | 16:49 |
zyga | pedronis: such samples are saved in /runs/snapd/perf/*.json and are loaded by the perf manager | 16:49 |
zyga | pedronis: either in ensure or when "snap debug speed" is used | 16:49 |
pedronis | zyga: that sounds delicate | 16:50 |
zyga | pedronis: that last command just shows collected samples and shows us data | 16:50 |
pedronis | because Ensure is in the hot path as well | 16:50 |
zyga | er, duration and meta-data | 16:50 |
zyga | it doesn't really need to be in ensure because it's collected on demand in the API debug call | 16:50 |
pedronis | except if you restart | 16:50 |
zyga | if the manager is stopped is saves the samples to /run/snapd/perf/ | 16:51 |
zyga | so on restart it can pick it up | 16:51 |
zyga | I haven't used that much though because for local testing with several snaps it wasn't the interesting case | 16:51 |
pedronis | zyga: I marked it as blocked, not because it not useful, I'm just worried about landing it in prod snapd for a while | 16:51 |
zyga | pedronis: sure | 16:52 |
zyga | pedronis: I will propose all the branches and collect them all in a evaluation branch for ongoing debugging | 16:52 |
pedronis | zyga: what's the state of per-user mounts? | 16:53 |
zyga | pedronis: under review, one branch needs my activity but master was wonky so I didn't push it forward yet | 16:54 |
pedronis | ok | 16:54 |
zyga | pedronis: I will fix that branch because the issue is small, just sequencing of other PRs | 16:54 |
zyga | pedronis: I really want to end that work :) | 16:54 |
* zyga -> break for tea | 16:56 | |
mup | PR 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 | ||
zyga | mvo: hey | 18:25 |
zyga | if you are still working I would love to review and land https://github.com/snapcore/snapd/pull/6181 | 18:25 |
mup | PR #6181: features: add feature test methods <Per-user mount ns 🐎> <Performance 🚀> <Created by zyga> <https://github.com/snapcore/snapd/pull/6181> | 18:25 |
zyga | as well as https://github.com/snapcore/snapd/pull/6170 | 18:26 |
mup | PR #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 |
zyga | I'm blocked by those two | 18:26 |
mup | PR snapcraft#2414 opened: yaml_utils: allow unicode while encoding <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2414> | 18:42 |
zyga | jdstrand: can you please look at 6182 | 19:40 |
ijohnson | zyga: I think jdstrand is out today | 19:40 |
zyga | bummer | 19:41 |
mup | PR 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 |
mup | PR 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!