| *** benschubert has quit IRC | 00:06 | |
| *** hasebastian has joined #buildstream | 01:32 | |
| *** tristan has joined #buildstream | 04:44 | |
| *** ChanServ sets mode: +o tristan | 04:44 | |
| *** tristan has quit IRC | 05:51 | |
| *** hasebastian has quit IRC | 06:11 | |
| *** tristan has joined #buildstream | 11:46 | |
| *** ChanServ sets mode: +o tristan | 11:46 | |
| *** tristan has quit IRC | 14:01 | |
| *** tristan has joined #buildstream | 14:49 | |
| *** ChanServ sets mode: +o tristan | 14:49 | |
| *** benschubert has joined #buildstream | 15:09 | |
| benschubert | tristan: nice change for the variables, I'll go through it probably tomorrow | 15:12 |
|---|---|---|
| benschubert | Have you managed to get any benchmarking done for it? | 15:12 |
| tristan | benschubert, commented on !1973 :) | 15:18 |
| tristan | Doesnt seem to dramatically affect performance | 15:18 |
| tristan | I think that changing `while queue: ... queue.pop()` for deque() type might improve performance too, but doing this with cdef (and `from libcpp.deque import deque`) appears pretty tricky | 15:19 |
| tristan | s/from .. import deque/from .. cimport deque/ | 15:20 |
| benschubert | mmh fair | 15:20 |
| benschubert | have you tried a big project with few variables also by any chance? Otherwise I can try that | 15:20 |
| tristan | The performance is either better or worse than master | 15:20 |
| tristan | depending on --format %{vars} (as commented)... no I didn't try further benchmarking | 15:21 |
| tristan | The error reporting is much improved for cyclic and undefined variables too | 15:21 |
| tristan | tox -e py37 -- -s tests/format/variables.py::test_circular_reference for a sample | 15:22 |
| benschubert | ok :) I'll have a look, thanks a lot | 15:23 |
| tristan | benschubert, or here is a sample: https://bpa.st/TZUA :) | 15:23 |
| benschubert | oh nice! And what happens for a -> b -> c -> a ? | 15:23 |
| tristan | That is such a case | 15:24 |
| tristan | Sorry, didn't collect every link in the chain | 15:24 |
| benschubert | ah ok, so we don't get the whole chain? | 15:24 |
| tristan | The detail mentions a direct reference, while the main message is where the error occurs and we know that (somewhere) there is a reference to the variable in the detail | 15:24 |
| tristan | Getting the whole chain might be plausible | 15:25 |
| benschubert | yeah It usually helps to get the head around | 15:25 |
| benschubert | Like a 10-ish variables cycle would be quite hard to understand otherwise | 15:25 |
| tristan | I'm not sure if it's more expensive, probably can do it by enhancing https://gitlab.com/BuildStream/buildstream/-/blob/tristan/partial-variables/src/buildstream/_variables.pyx#L301 | 15:26 |
| tristan | Such that we store more information in the dict | 15:26 |
| tristan | Currently I use a dict of sets, where each entry is a variable name, and a set of direct references from that variable | 15:26 |
| tristan | e.g. "foo: "%{bar}, %{baz}" is { 'foo': { 'bar', 'baz' } } | 15:27 |
| benschubert | tristan: if that means having to do a second pass on a cycle, it's fine too? We might have like a "error handling" one that is slower but keeps all the info | 15:27 |
| benschubert | and do the minimal in the happy case | 15:27 |
| tristan | Possible also | 15:27 |
| tristan | More wordy | 15:27 |
| tristan | I mean, currently it certainly better than: https://gitlab.com/BuildStream/buildstream/-/blob/master/src/buildstream/_variables.pyx#L209 | 15:28 |
| tristan | but I'll look into a full chain too since my head is close by (not tonight, perhaps tomorrow) | 15:29 |
| benschubert | thanks :) | 15:31 |
| tristan | it was an interesting brain exercise to convert recursive stuff to queues :) | 15:31 |
| * tristan off for midnight snack... | 15:32 | |
| *** jward has joined #buildstream | 22:08 | |
| *** benschubert has quit IRC | 23:29 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!