| *** tristan has quit IRC | 05:06 | |
| *** tristan has joined #buildstream | 06:13 | |
| *** ChanServ sets mode: +o tristan | 06:13 | |
| *** benschubert has joined #buildstream | 07:21 | |
| benschubert | tristan, juergbi : seems like threading is not yet that fast. Initial benchmarks are twice slower on the debian project. | 07:22 |
|---|---|---|
| benschubert | This brings me to a question: How/when would we be happy moving on with the threaded implementation? Should we do like for the yaml refactor, a long lived branch that gradually shapes and improve the code until we are happy? | 07:22 |
| *** tomaz has joined #buildstream | 07:28 | |
| tristan | benschubert, What are the prospects and hotspots here ? plugins walking dependency trees and such ? | 07:48 |
| benschubert | tristan: | 07:50 |
| benschubert | - the event loop polling too much?? (85s (half of the time) | 07:50 |
| benschubert | - The child-parent split in Job that's wreaking havoc for nothing | 07:50 |
| benschubert | Are the two biggest hotspots I can see | 07:50 |
| tristan | Maybe we need to go the pickle route (also eliminating the `multiprocessing` module but replacing with subprocesses) after all ? what is more promising ? | 07:50 |
| benschubert | I'll be removing the second as soon as all the tests pass and I have a clean commit | 07:50 |
| benschubert | I think the threading route might give us nicer code and handling | 07:50 |
| tristan | How is parent/child in Job "for nothing" ? | 07:50 |
| benschubert | so I think it's worth continuing for a while (at least until "clean"implementation + a few optis) | 07:50 |
| tristan | Ah I guess it would go away with threading ? | 07:51 |
| benschubert | yep exactly | 07:51 |
| benschubert | with threading it needs to go away (all the communication got hrough a pipe, which is not needed and puts burden on the epoll()) | 07:51 |
| tristan | Right, and with subprocessing and pickling of the minimal amount of state, we don't have the issue of worrying about the GIL at all | 07:52 |
| benschubert | so: I think it can be done and we can have as good perf as with multiprocess. However, I think we would need a big MR to have it nice and avoid mistakes :) | 07:52 |
| benschubert | tristan: that's also true | 07:52 |
| tristan | I mean, threading approach assumes there is no justification of in-process python computation | 07:53 |
| tristan | And I don't know if I can believe that | 07:53 |
| tristan | In the long term, it may add unnecessary onus on optimizing the python codepaths | 07:53 |
| benschubert | we have the computing of cache keys and whether things are cached that takes time | 07:54 |
| tristan | And walking the dependency tree | 07:54 |
| tristan | For implementing stage_artifact() and stage_dependency_artifacts() | 07:54 |
| tristan | And for implementing Element.search(), for whatever reason an Element might use that for | 07:54 |
| benschubert | stage_Artifact is mostly buildbox-casd no? | 07:55 |
| benschubert | through grpc | 07:55 |
| tristan | Regardless of how fast we can get staging to happen in the casd, the python side needs to be allowed to select which elements get staged where | 07:55 |
| tristan | That implies a plugin/buildstream side walk anyway | 07:55 |
| benschubert | true | 07:56 |
| tristan | That's an important feature (I should be able to say, I want the 'foo' domain of dependency 'bar.bst', without it's deps, in directory '/baz') | 07:56 |
| tristan | Maybe there will be more I dont know | 07:56 |
| benschubert | that's not that big computationally though right? | 07:56 |
| tristan | Of course, in C we would be able to construct this lock free, as most of the data model is immutable once processing starts | 07:57 |
| tristan | but I'm not sure we can in python | 07:57 |
| tristan | I don't know | 07:57 |
| tristan | benschubert, it could be slow with very wide dependency graphs and lots of workers in RE | 07:57 |
| benschubert | I mean we have two things: | 07:57 |
| benschubert | - creating a subprocess is not free, and with pickling it will take some noticeable time | 07:57 |
| benschubert | - compute-intensive stuff in python | 07:57 |
| benschubert | And we need to bet on one ? :) | 07:57 |
| tristan | the bottleneck would become BuildStream python | 07:57 |
| WSalmon | +11111 <tristan> That's an important feature (I should be able to say, I want the 'foo' domain of dependency 'bar.bst', without it's deps, in directory '/baz') | 07:57 |
| tristan | benschubert, exactly | 07:58 |
| WSalmon | or everything but the licence domain then have all the licence domains somewere else | 07:58 |
| tristan | benschubert, that is a hard problem, then again with subprocesses, process launch overhead could be constant with some reuse of pids | 07:58 |
| benschubert | tristan: you mean a jobserver? :) Yeah that would be not too bad either | 07:59 |
| tristan | benschubert, pickling is problematic, ideally we don't *really* have to pickle, and can use only custom Queues and proper internal IPC apis | 07:59 |
| tristan | simple objects | 07:59 |
| tristan | Child processes can instantiate plugins on demand just to execute tasks, etc | 07:59 |
| tristan | but that's a lot of internal API craftmanship | 07:59 |
| benschubert | tristan: and if you have a plugin that needs it's whole dep tree? What now? | 07:59 |
| benschubert | that's a lot of data across | 08:00 |
| tristan | Ah right, good point | 08:00 |
| benschubert | So yeah, I don't think there is a right solution there | 08:00 |
| tristan | Well | 08:00 |
| tristan | I mean | 08:00 |
| benschubert | (Actually, all plugins will want their dep tree, just to be able to stage) | 08:00 |
| tristan | Lemme think, we had a concept at some point that there would be an ArtifactElement, e.g. an Element instantiated form an artifact | 08:01 |
| tristan | When processing an Element, we should not need the real plugin type instantiated for dependencies should we ? Maybe we do I'm not 100% certain | 08:01 |
| benschubert | We might? Bazel element? ;) | 08:01 |
| tristan | But ensuring that a plugin only ever has a minimal Element implementation for it's dependencies when processing, would eliminate any abuses such as issue #3 of bst-plugins-external | 08:02 |
| tristan | Why would we require an instantiated BazelElement as a dependency ? | 08:02 |
| tristan | We only need it's output to be staged, the ability to stage it's output | 08:02 |
| tristan | and if the current element is a BazelElement, it should have the capabilities it needs | 08:03 |
| juergbi | it might be interesting to benchmark a more realistic project | 08:03 |
| benschubert | tristan: bazel element might want to look at the sources potentially, but yeah | 08:04 |
| benschubert | juergbi: agreed | 08:04 |
| juergbi | I'm definitely still leaning towards the threaded approach | 08:04 |
| tristan | benschubert, right, and that is the kind of abuse in bst-plugins-external we would eliminate | 08:04 |
| tristan | benschubert, the model would force BazelElement to not cheat, and implement things in core properly instead :D | 08:04 |
| juergbi | should overall be simpler than pickling, imo | 08:04 |
| benschubert | tristan: Not speaking about bst-plugins-external there, but rather the idea of a nice transparent bazel plugin :) | 08:04 |
| juergbi | and I expect that we can get it perform reasonably well | 08:04 |
| tristan | Of course, you need real instantiation of your own sources in order to stage them | 08:05 |
| benschubert | juergbi: I too lean towards threading, it is not necessarily simpler though, we had assumptions about what would change where, and many break now | 08:05 |
| tristan | And any context you get from Source API (directly of via dependencies), needs to be transported via BuildStream core API in some way anyway | 08:05 |
| tristan | But anyway, this is far down the rabbit hole for a theory :) | 08:05 |
| juergbi | I'll have to look at the changes to get a better idea | 08:07 |
| benschubert | Yep. I'll make a "minimal" POC PR as soon as I get all tests pass reliably | 08:07 |
| juergbi | thanks | 08:07 |
| juergbi | benschubert: regarding potential merge, the branch completely replaces the subprocess model, right? or could it be feasible/sensible to support both at once in a single branch? | 08:08 |
| benschubert | I don't think supporting both is feasible in an acceptable way | 08:08 |
| juergbi | ok | 08:09 |
| benschubert | juergbi: if you are interested current set of changes is https://gitlab.com/BuildStream/buildstream/-/compare/master...bschubert%2Fno-multiprocessing | 08:09 |
| benschubert | but there are things that I'll have to remove because they are actually needed | 08:10 |
| juergbi | ta | 08:12 |
| *** phildawson has joined #buildstream | 08:20 | |
| *** santi has joined #buildstream | 08:27 | |
| tristan | benschubert, want to try the new tristan/partial-variables ? | 08:48 |
| tristan | It should be faster now | 08:48 |
| tristan | maybe not quite there yet | 08:48 |
| tristan | but faster, would be interested to see your bench | 08:48 |
| tomaz | it's possible to use buildstream as build tool, like `kdesrc-build`? (downloads / build / install software locally, using local libraries if they are installed?) | 08:50 |
| tomaz | if this works I'll try to use buildstream to replace kdesrc-build besides the flatpak and snap generation. | 08:50 |
| tristan | tomaz, no | 08:56 |
| tomaz | dang. | 08:57 |
| tristan | tomaz, rather the whole point is to be completely isolated from your host system (so developers working on system 'foo' are using exactly the same dependencies and produce exactly the same results, regardless of their host) | 08:57 |
| tristan | in a FOSDEM talk a few years ago, I did do a demonstration of staging debian base system as a dependency in BuildStream, to build a debian package targetting that debian system, and then check it out locally and install it on my host | 08:59 |
| tristan | because my host happened to be the same debian version | 08:59 |
| tristan | but I should be able to build for any debian version on my fedora or any host | 08:59 |
| tomaz | yeah, I just tought it could be a good way to replace gnome-builder or kdesrc-build if it could target the same system. (for instance, I don't want to rebuild qt when I'm building KDE software that I use locally to test) | 09:00 |
| tomaz | maybe for the distant future. :) | 09:00 |
| coldtom | it may be possible to do local dev by leveraging workspaces and bst shell, but i've never tried it in anger | 09:03 |
| tristan | It's made for that, but requires that your project.conf declares some shell hints | 09:05 |
| tristan | tomaz, https://docs.buildstream.build/master/format_project.html#customizing-the-shell | 09:08 |
| tristan | tomaz, https://gitlab.gnome.org/GNOME/gnome-build-meta/-/blob/master/project.conf#L157 | 09:09 |
| tristan | For some reason, freedesktop-sdk still didnt pick up on customizing the shell | 09:09 |
| tristan | tomaz, that should work for your basic linux applications which need pulseaudio and graphics acceleration | 09:10 |
| tristan | tomaz, Of course, this is supposed to be *better* than developping on your host | 09:10 |
| tristan | tomaz, because you can for example, make a build of firefox, and then open a workspace on glibc, and use `bst build --no-strict` | 09:10 |
| tristan | So you should be able to make minor tweaks to lower level dependency libraries and see how that impacts a higher level graphics application | 09:11 |
| tomaz | tristan: aha. | 09:11 |
| benschubert | tristan: let me try :) | 09:31 |
| tristan | benschubert, iterating over lists, and runtime checks on strings, appear to be a big overhead here; creating C arrays of C strings might be an option, but would probably mean "".join(list) in C and a final call to PyUnicode_DecodeUTF8() | 09:43 |
| benschubert | tristan: cython can convert a c string to python automatically if you return it | 09:43 |
| tristan | And ideally it would mean using C strings as dictionary keys | 09:43 |
| benschubert | so not even need to do this :) | 09:43 |
| tristan | Oh ? | 09:43 |
| benschubert | yep! | 09:43 |
| tristan | Interesting | 09:43 |
| tristan | I wasn't sure if I was going to take it that far, but could be a good idea | 09:44 |
| tristan | think I can just use a C string as a dictionary key ? Maybe cast it as <void *> ? | 09:44 |
| tristan | The strings are interned anyway and the memory they point to is still referenced by the original string objects | 09:45 |
| benschubert | yeah I'm not sure we'd win a lot there | 09:46 |
| benschubert | wonder if we could do this instead: | 09:46 |
| benschubert | ``` | 09:46 |
| benschubert | cdef object string_temp | 09:46 |
| benschubert | cdef str string | 09:46 |
| benschubert | for string_temp in my_list: | 09:46 |
| benschubert | string = <str> string_temp | 09:46 |
| benschubert | ``` | 09:46 |
| benschubert | like trick cython to generate code without the checks? | 09:47 |
| tristan | I don't get it | 09:47 |
| tristan | Oh | 09:47 |
| tristan | Really ? | 09:47 |
| tristan | Well | 09:48 |
| benschubert | worth a try to see what cython generates? | 09:48 |
| tristan | benschubert, there are two parts to this, one of them is having to have a list at all | 09:48 |
| benschubert | and that would probably then be optimized by the ocmpiler | 09:48 |
| tristan | cdef char **strings = malloc(sizeof(void *) * len(string_list)) | 09:49 |
| tristan | and then while loops with indexes into the string array | 09:49 |
| tristan | I think is much faster | 09:49 |
| benschubert | worth a try :) | 09:50 |
| tristan | There are some parts where sets are useful though, e.g. in the cyclic dependency check queue, we do set_object.update(dependencies) | 09:51 |
| tristan | tradeoffs | 09:51 |
| tristan | benschubert, nice trick | 09:54 |
| benschubert | the double def? :) | 09:54 |
| benschubert | this is something that's nowhere in optimizations trick with cython | 09:58 |
| benschubert | /me should start a blog around optimizing python tricks | 09:58 |
| tristan | yeah | 09:58 |
| tristan | double def works | 09:59 |
| tristan | helps at least | 09:59 |
| benschubert | :D let me know when you have a branch I should re-benchmark. the previous one is running currently | 09:59 |
| tristan | So is it that the <typecast> syntax does not incur a runtime typecheck ? | 09:59 |
| benschubert | and if you want to see per-commit you made since master I can also provide | 09:59 |
| benschubert | correct | 09:59 |
| benschubert | it's like a c cast | 09:59 |
| tristan | Ok, I wouldn't have guessed | 10:00 |
| benschubert | so you tell cyhton, I know what this is, trust me and segfault | 10:00 |
| benschubert | minus the segfault if you are actually right :) | 10:00 |
| tristan | Hmmm, method calls are given tuples | 10:10 |
| tristan | at least constructors | 10:11 |
| tristan | I'd have expected function specific structs | 10:11 |
| Frazer | hi, are we still using the old Mailing List to discuss implementing new features, or should we move over to the new one now? | 10:15 |
| tristan | Just move to the new one | 10:15 |
| tristan | Frazer, ^^ | 10:15 |
| Frazer | thanks, just to confirm is it this one in the irc topic? https://lists.apache.org/list.html?dev@buildstream.apache.org | 10:16 |
| tristan | Yes. | 10:16 |
| Frazer | thanks | 10:16 |
| tristan | benschubert, do you know if it is worth that same trick on Cython objects you declare ? I guess so ? | 10:18 |
| tristan | for value_part_object in self._value_parts: | 10:18 |
| benschubert | probably yes :) | 10:18 |
| tristan | value_part = <ValuePart> value_part_object | 10:18 |
| benschubert | we can probably do this and benchmark | 10:19 |
| tristan | I think that I'm at the limit without getting into real arrays | 10:19 |
| tristan | except for one thing | 10:19 |
| tristan | https://gitlab.com/BuildStream/buildstream/-/blob/tristan/partial-variables/src/buildstream/_variables.pyx#L247 and https://gitlab.com/BuildStream/buildstream/-/blob/tristan/partial-variables/src/buildstream/_variables.pyx#L351 | 10:20 |
| tristan | benschubert, looking at those two `try` lines | 10:21 |
| tristan | benschubert, I think it's possible to remove one dictionary lookup by making the second try loop responsible for raising UNDEFINED_VARIABLE error | 10:21 |
| tristan | maybe change _check_circular_references() for something like self._validate(), and have it return a Value | 10:22 |
| tristan | I think if I can do that, I'll be at a point where the cython is as good as it gets, I can save that branch as a reference point, and then do a C string/array exercise after | 10:23 |
| benschubert | that seems great | 10:24 |
| benschubert | if you separate commits nicely I cna also give you the numbers per commit | 10:24 |
| benschubert | if we want veri fine-grained knowledge | 10:24 |
| tristan | Hmmm, maybe I'll start with those two steps | 10:24 |
| tristan | Separate commit for stringification | 10:24 |
| tristan | Did the ongoing benchmark complete ? | 10:24 |
| benschubert | not yet | 10:25 |
| tristan | Ok I think I'll get some food and come back. | 10:25 |
| benschubert | ok! | 10:25 |
| tristan | The full stack circular reference error provenance thing will be very nice and easy to do, but I'm leaving that pleasure for the end | 10:26 |
| *** tristan has quit IRC | 10:29 | |
| benschubert | tristan: https://gitlab.com/snippets/1991367 current state | 11:01 |
| *** tristan has joined #buildstream | 11:10 | |
| *** ChanServ sets mode: +o tristan | 11:10 | |
| tristan | back... benschubert any half decent news ? | 11:13 |
| benschubert | tristan: https://gitlab.com/snippets/1991367 current state | 11:13 |
| benschubert | so based on the `show` it seemsslightly better | 11:14 |
| tristan | grumble | 11:16 |
| benschubert | mmh interesting, we have roughly 3000 DeprecationWarnings that appeared after running the code under the new scheduler, turns out we have code that raised warning but never caught since running only in the sub processes :/ | 11:39 |
| tristan | crap | 12:41 |
| tristan | Every once and a while, you paint yourself into a corner where cython screws up | 12:41 |
| tristan | Like AttributeError: 'buildstream._variables.Value' object has no attribute 'provenance' | 12:41 |
| tristan | Or you name a list with the same name as something else, and the code generator bails out with an internal stack trace not telling you what really happened | 12:43 |
| tristan | and then you have to go add sys.stderr.write() statements in .tox/.../CythonCodeGeneratorStuff/... | 12:43 |
| benschubert | :/ | 12:52 |
| benschubert | that's one reason why I think we need not to add too much of it, only where it brings a lot :) | 12:52 |
| tristan | :-/ | 12:54 |
| * tristan rewinds and makes tiny incrememtal changes with his patch | 12:55 | |
| tristan | Another, perhaps saner plan, is to create C modules and only bind to them in python | 12:55 |
| tristan | That way you could perhaps use normal tooling to debug C problems | 12:55 |
| tristan | I guess that would have to start with a faster, C implementation of ruamel.yaml | 12:56 |
| benschubert | ruamel.yaml.clib is in cython | 12:59 |
| benschubert | I wanted to have them export their symbols | 12:59 |
| benschubert | but the project is in an aweful limbo right now | 12:59 |
| benschubert | the alternative would be to use libyaml directly | 12:59 |
| benschubert | but then, it might throw off contributions and make buildstream harder to install | 13:00 |
| tristan | is libyaml roundtripping ? | 13:13 |
| benschubert | oh I don't think so | 13:14 |
| benschubert | it's true, we need roundtrip -_-' | 13:14 |
| tristan | I think I've got something closer... need to polish up something commit worthy | 13:16 |
| tristan | Oh damn no, that wont work :'( | 13:19 |
| * tristan knows that he is missing something simple | 13:29 | |
| tristan | The nature of the loop iterating over dependencies should allow for a more rudimentary cyclic dependency, which can be followed by the more loopy reporting code | 13:30 |
| tristan | I need a counter ! | 13:30 |
| tristan | Where does it go | 13:31 |
| tristan | So basically, I have a set of already traversed variables which are "safe so far", and if they ever intersect with the next level of indirection (variables they refer to), then there is a circular dependency | 13:32 |
| tristan | But given that you can have "%{foo} and %{foo}", you need to iterate twice on that level before checking for circular refs | 13:32 |
| tristan | or rather you can have %{foo} and %{bar} both use %{baz}, but you will process %{foo} -> %{baz} before encountering %{bar}, without a counter, you don't know that %{bar} doesnt constitute a circular ref | 13:33 |
| tristan | Hmmmm | 13:33 |
| * tristan thinks a walk and a breather will fix it | 13:34 | |
| WSalmon | so it turns out that changing the format-version in project.conf of bst-1.4 changes the cache keys of some of the elements in a junction but not all of them! | 13:49 |
| tristan | That is a bug if it happens | 13:51 |
| tristan | I seem to recall some people had some hare brained ideas, thinking that code should change behavior depending on format-version | 13:52 |
| tristan | it's possible that some madness slipped in while I was away ? | 13:52 |
| WSalmon | tristan, should be pretty eash to reproduce, just toggle the version and run show https://gitlab.com/willsalmon/busybox-system/-/commit/cf139114c2c44762d5bfa45de0eb026942a9e4c5#bb485c194054386656d16ab113f2b2fe87c99e3e_6_5 | 13:54 |
| WSalmon | waiting 396f85cb20c1f24706a7c9fa2df6ca15ac9320ba404ab465e5d0f112c1c0242a freedesktop-sdk.bst:bootstrap-import.bst vs waiting f253e43b797028413c2cb9ffa82f3573923ae13fc4654db18c47a2a7efc0abe9 freedesktop-sdk.bst:bootstrap-import.bst | 13:56 |
| WSalmon | fetch needed 12c68a5c5a294f373474813406545d25085c16870a4e402d289a1b83b8123a8b freedesktop-sdk.bst:bootstrap/build/base-sdk/image-x86_64.bst vs fetch needed 12c68a5c5a294f373474813406545d25085c16870a4e402d289a1b83b8123a8b freedesktop-sdk.bst:bootstrap/build/base-sdk/image-x86_64.bst | 13:57 |
| tristan | Doesnt appear possible | 13:58 |
| tristan | _project.py on 1.4 branch parses format-version and discards it immediately after performing a check | 13:58 |
| tristan | WSalmon, of course the commit you are referring to also changes a commit sha, which would *definitely* affect cache key | 13:59 |
| WSalmon | yep but i just commited everything needed to fix the cache keys not matching | 13:59 |
| WSalmon | i needed both changes | 14:00 |
| tristan | Yeah, but I don't see how format-version can possibly affect cache key in 1.4, it's not stored in context anywhere | 14:00 |
| tristan | code cannot be conditional on it, because it's not there | 14:00 |
| tristan | If you can provide an isolated test case that proves it, that would be helpful in an issue report | 14:00 |
| WSalmon | ummm, i have just toggled them and it dose look like its not the issue, but im really confused as when i updated the junction it didnt seem to change it.. im really confused | 14:02 |
| *** tristan has quit IRC | 14:04 | |
| *** tomaz has quit IRC | 17:41 | |
| *** santi has quit IRC | 17:47 | |
| *** tristan has joined #buildstream | 18:04 | |
| *** ChanServ sets mode: +o tristan | 18:04 | |
| *** benschubert has quit IRC | 22:20 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!