| Age | Commit message (Collapse) | Author |
|---|
|
goals should be considered internal to the worker architecture due to
the tight coupling of the two, and we can finally do that. doing this
is also a prerequisite for turning Worker::run() into a real promise.
Change-Id: I7cf273d4a6fdb75b8d192fce1af07c6265ff6980
|
|
we already have a results type for the entire worker call, we may as
well put this bit of info in there. we do keep the assumption of the
old code that the field will only be read if some goals have failed;
fixing that is a very different mess, and not immediately necessary.
Change-Id: If3fc32649dcd88e1987cdd1758c6c5743e3b35ac
|
|
While debugging something else I observed that latest `main` ignores
`Control-C` on `sudo nix-build`.
After reading through the capnproto docs, it seems as if the promise
must be fulfilled to actually terminate the `promise.wait()` below.
This also applies to scenarios such as stopping the client
(`nix-build`), but the builders on the daemon-side are still running,
i.e. closes #540
Co-authored-by: eldritch horrors <pennae@lix.systems>
Change-Id: I9634d14df4909fc1b65d05654aad0309bcca8a0a
|
|
Add the compile flag '-Wsign-compare' and adapt the code to fix all
cases of this warning.
Change-Id: I26b08fa5a03e4ac294daf697d32cf9140d84350d
|
|
these two functions are now nearly trivial and much better inline into
makeGoalCommon. keeping them separate also separates information about
goal completion flows and how failure information ends up in `Worker`.
Change-Id: I6af86996e4a2346583371186595e3013c88fb082
|
|
we can use our newfound powers of Goal::work Is A Real Promise to remove
completed goals from continuation promises. apart from being much easier
to follow it's also a lot more efficient because we have the iterator to
the item we are trying to remove, skipping a linear search of the cache.
Change-Id: Ie0190d051c5f4b81304d98db478348b20c209df5
|
|
Goal::work() is a fully usable promise that does not rely on the worker
to report completion conditions. as such we no longer need the `notify`
field that enabled this interplay. we do have to clear goal caches when
destroying the worker though, otherwise goal promises may (incorrectly)
keep goals alive due to strong shared pointers created by childStarted.
Change-Id: Ie607209aafec064dbdf3464fe207d70ba9ee158a
|
|
this will be needed to move all interesting result fields out of Goal
proper and into WorkResult. once that is done we can treat goals as a
totally internal construct of the worker mechanism, which also allows
us to fully stop exposing unclear intermediate state to Worker users.
Change-Id: I98d7778a4b5b2590b7b070bdfc164a22a0ef7190
|
|
since we now propagate goal exceptions properly we no longer need to
check topGoals for a reason to abort early. any early abort reasons,
whether by exception or a clean top goal failure, can now be handled
by inspecting the goal result in the main loop. this greatly reduces
goal-to-goal interactions that do not happen at the main loop level.
since the underscore-free name is now available for use as variables
we'll migrate to that where we currently use `_topGoals` for locals.
Change-Id: I5727c5ea7799647c0a69ab76975b1a03a6558aa6
|
|
drop childException since it's no longer needed. also makes
waitForInput, childFinished, and childTerminated redundant.
Change-Id: I05d88ffd323c5b5c909ac21056162f69ffb0eb9f
|
|
Change-Id: Idd218ec1572eda84dc47accc0dcd8a954d36f098
|
|
there's no reason to have the worker set information on goals that the
goals themselves return from their entry point. doing this in the goal
`work()` function is much cleaner, and a prerequisite to removing more
implicit strong shared references to goals that are currently running.
Change-Id: Ibb3e953ab8482a6a21ce2ed659d5023a991e7923
|
|
this simplifies the worker loop, and lets us remove it entirely later.
note that ideally only one promise waiting for interrupts should exist
in the entire system. not one per event loop, one per *process*. extra
interrupt waiters make interrupt response nondeterministic and as such
aren't great for user experience. if anything wants to react to aborts
caused by explicit interruptions, or anything else, those things would
be much better served using RAII guards such as Finally (or KJ_DEFER).
Change-Id: I41d035ff40172d536e098153c7375b0972110d51
|
|
this was a triumph. i'm making a note here: huge success. it's hard to
overstate my satisfaction! i'm not even angry. i'm being so sincere ri
actually, no. we *are* angry. this was one dumbass odyssey. nobody has
asked for this. but not doing it would have locked us into old, broken
protocols forever or (possibly worse) forced us to write our own async
framework building on the old did-you-mean-continuations in Worker. if
we had done that we'd be locked into ever more, and ever more complex,
manual state management all over the place. this just could not stand.
Change-Id: I43a6de1035febff59d2eff83be9ad52af4659871
|
|
due to event loop scheduling behavior it's possible for a derivation
goal to fully finish (having seen all paths it was asked to create),
but to not notify the worker of this in time to prevent another goal
asking the recently-finished goal for more outputs. if this happened
the finished goal would ignore the request for more outputs since it
considered itself fully done, and the delayed result reporting would
cause the requesting goal to assume its request had been honored. if
the requested goal had finished *properly* the worker would recreate
it instead of asking for more outputs, and this would succeed. it is
thus safe to always recreate goals once they are done, so we now do.
Change-Id: Ifedd69ca153372c623abe9a9b49cd1523588814f
|
|
we'll now loop to update displayed statistics, and use this loop to
limit the update rate to 50 times per second. we could have updated
much more frequently before this (once per iteration of `runImpl`),
much faster than would ever be useful in practice. aggressive stats
updates can even impede progress due to terminal or network delays.
Change-Id: Ifba755a2569f73c919b1fbb06a142c0951395d6d
|
|
Worker::run() is now entirely based on the kj event loop and promises,
so we need not handle awakeness of goals manually any more. every goal
can instead, once it has finished a partial work call, defer itself to
being called again in the next iteration of the loop. same end effect.
Change-Id: I320eee2fa60bcebaabd74d1323fa96d1402c1d15
|
|
notably we will check whether we want to do GC at all only once during
startup, and we'll only attempt GC every ten seconds rather than every
time a goal has finished a partial work call. this shouldn't cause any
problems in practice since relying on auto-gc is not deterministic and
stores in which builds can fill all remaining free space in merely ten
seconds are severely troubled even when gargage collection runs a lot.
Change-Id: I1175a56bf7f4e531f8be90157ad88750ff2ddec4
|
|
Change-Id: Ib112ea9a3e67d5cb3d7d0ded30bbd25c96262470
|
|
Change-Id: I8355d8d3f6c43a812990c1912b048e5735b07f7b
|
|
Revert submission 1946
Reason for revert: regression in building (found via bisection)
Reported by users:
> error: path '/nix/store/04ca5xwvasz6s3jg0k7njz6rzi0d225w-jq-1.7.1-dev' does not exist in the store
Reverted changes: /q/submissionid:1946
Change-Id: I6f1a4b2f7d7ef5ca430e477fc32bca62fd97036b
|
|
nothing needs to signal being still active but not actively pollable,
only that immediate polling for the next goal work phase is in order.
Change-Id: Ia43c1015e94ba4f5f6b9cb92943da608c4a01555
|
|
this was immensely inefficient on large caches, as can exist when many
derivations are buildable simultaneously. since we have smart pointers
to goals we can do cache maintenance in goal deleters instead, and use
the exact iterators instead of doing a linear search. this *does* rely
on goals being deleted to remove them from the cache, which isn't true
for toplevel goals. those would have previously been removed when done
in all cases, removing the cache entry when keep-going is set. this is
arguably incorrect since it might result in those goals being retried,
although that could only happen with dynamic derivations or the likes.
(luckily dynamic derivations not complete enough to allow this at all)
Change-Id: I8e750b868393588c33e4829333d370f2c509ce99
|
|
makeDerivationGoalCommon had the right idea, but it didn't quite go far
enough. let's do the rest and remove the remaining factory duplication.
Change-Id: I1fe32446bdfb501e81df56226fd962f85720725b
|
|
without circular references we do not need weak goal pointers except for
caches, which should not prevent goal destructors running. caches though
cannot create circular references even when they keep strong references.
if we removed goals from caches when their work() is fully finished, not
when their destructors are run, we could keep strong pointers in caches.
since we do not gain much from this we keep those pointers weak for now.
Change-Id: I1d4a6850ff5e264443c90eb4531da89f5e97a3a0
|
|
have DerivationGoal and its subclasses produce a wrapper promise for
their intermediate results instead, and return this wrapper promise.
Worker already handles promises that do not complete immediately, so
we do not have to duplicate this into an entire result type variant.
Change-Id: Iae8dbf63cfc742afda4d415922a29ac5a3f39348
|
|
these can be unique rather than shared because shared_ptr has a
converting constructor. preparatory refactor for something else
and not necessary on its own, and the extra allocations we must
do for shared_ptr control blocks isn't usually relevant anyway.
Change-Id: I5391715545240c6ec8e83a031206edafdfc6462f
|
|
also gets rid of explicit strong references to dependencies of any goal,
and weak references to dependers as well. those are now only held within
promises representing goal completion and thus independent of the goal's
relation to each other. the weak references to dependers was only needed
for notifications, and that's much better handled entirely by kj itself.
Change-Id: I00d06df9090f8d6336ee4bb0c1313a7052fb016b
|
|
now that we have an event loop in the worker we can use it and its
magical execution suspending properties to replace the slot counts
we managed explicitly with semaphores and raii tokens. technically
this would not have needed an event loop base to be doable, but it
is a whole lot easier to wait for a token to be available if there
is a callback mechanism ready for use that doesn't require a whole
damn dedicated abstract method in Goal to work, and specific calls
to that dedicated method strewn all over the worker implementation
Change-Id: I1da7cf386d94e2bbf2dba9b53ff51dbce6a0cff7
|
|
with waitForAWhile turned into promised the core functionality of
waitForInput is now merely to let gc run every so often if needed
Change-Id: I68da342bbc1d67653901cf4502dabfa5bc947628
|
|
this simplifies waitForInput quite a lot, and at the same time makes
polling less thundering-herd-y. it even fixes early polling wakeups!
Change-Id: I6dfa62ce91729b8880342117d71af5ae33366414
|
|
this removes the rather janky did-you-mean-async poll loop we had so
far. sadly kj does not play well with pty file descriptors, so we do
have to add our own async input stream that does not eat pty EIO and
turns it into an exception. that's still a *lot* better than the old
code, and using a real even loop makes everything else easier later.
Change-Id: Idd7e0428c59758602cc530bcad224cd2fed4c15e
|
|
using a proper event loop basis we no longer have to worry about most of
the intricacies of poll(), or platform-dependent replacements for it. we
may even be able to use the event loop and its promise system for all of
our scheduling in the future. we don't do any real async processing yet,
this is just preparation to separate the first such change from the huge
api design difference with the async framework we chose (kj from capnp):
kj::Promise, unlike std::future, doesn't return exceptions unmangled. it
instead wraps any non-kj exception into a kj exception, erasing all type
information and preserving mostly the what() string in the process. this
makes sense in the capnp rpc use case where unrestricted exception types
can't be transferred, and since it moves error handling styles closer to
a world we'd actually like there's no harm in doing it only here for now
Change-Id: I20f888de74d525fb2db36ca30ebba4bcfe9cc838
|
|
the more useful type for `result` is BuildResult.
Change-Id: If93d9384e8d686eb63b33320f1d565f9b9afbf3a
|
|
it doesn't have a purpose except cache priming, which is largely
irrelevant by default (since another code path already runs this
exact query). our store implementations do not benefit that much
from this either, and the more bursty load may indeed harm them.
Change-Id: I1cc12f8c21cede42524317736d5987f1e43fc9c9
|
|
updating statistics *immediately* when any counter changes declutters
things somewhat and makes useful status reports less dependent on the
current worker main loop. using callbacks will make it easier to move
the worker loop into kj entirely, using only promises for scheduling.
Change-Id: I695dfa83111b1ec09b1a54cff268f3c1d7743ed6
|
|
this is not ideal, but it's better than having this stuck in the worker
loop itself. setting ex on all failing goals is not problematic because
only toplevel goals can ever be observable, all the others are ignored.
notably only derivation goals ever set `ex`, substitution goals do not.
Change-Id: I02e2164487b2955df053fef3c8e774d557aa638a
|
|
this doesn't serve a great purpose yet except to confine construction of
goals to the stack frame of Worker::run() and its child frames. we don't
need this yet (and the goal constructors remain fully visible), but in a
future change that fully removes the current worker loop we'll need some
way of knowing which goals are top-level goals without passing the goals
themselves around. once that's possible we can remove visible goals as a
concept and rely on build result futures and a scheduler built upon them
Change-Id: Ia73cdeffcfb9ba1ce9d69b702dc0bc637a4c4ce6
|
|
whether goal errors are reported via the `ex` member or just printed to
the log depends on whether the goal is a toplevel goal or a dependency.
if goals are aware of this themselves we can move error printing out of
the worker loop, and since a running worker can only be used by running
goals it's totally sufficient to keep a `Worker::running` flag for this
Change-Id: I6b5cbe6eccee1afa5fde80653c4b968554ddd16f
|
|
this makes WorkResult copyable, and just all around easier to deal with.
in the future we'll need this to let Goal::work() return a promise for a
WorkResult (or even just a Finished) that can be awaited by other goals.
Change-Id: Ic5a1ce04c5a0f8e683bd00a2ed2b77a2e28989c1
|
|
this should be done where we're actually trying to build something, not
in the main worker loop that shouldn't have to be aware of such details
Change-Id: I07276740c0e2e5591a8ce4828a4bfc705396527e
|
|
this can be a proper WorkResult now. childTerminated is unfortunately a
lot more stubborn and won't be made private for quite a while yet. once
we can get rid of the Worker poll loop that *should* be possible though
Change-Id: I2218df202da5cb84e852f6a37e4c20367495b617
|
|
this is useless to do on the face of it, but it'll make it easier to
convert the entire output handling to use async io and promises soon
Change-Id: I2d1eb62c4bbf8f57bd558b9599c08710a389b1a8
|
|
we don't need to expose information about how busy a Worker is if the
worker can instead tell its work items whether they are in a slot. in
the future we might use this to not start items waiting for a slot if
no slots are currently available, but that requires more preparation.
Change-Id: Ibe01ac536da7e6d6f80520164117c43e772f9bd9
|
|
* changes:
clang-tidy: write a lint for charptr_cast
tree-wide: automated migration to charptr_cast
clang-tidy: enforce the new rules
|
|
* changes:
sqlite: add a Use::fromStrNullable
util: implement charptr_cast
tree-wide: fix a pile of lints
refactor: make HashType and Base enum classes for type safety
build: integrate clang-tidy into CI
|
|
The lint did it :3
Change-Id: I2d9f276b01ebbf14101de4257ea13e44ff6fe0a0
|
|
This:
- Converts a bunch of C style casts into C++ casts.
- Removes some very silly pointer subtraction code (which is no more or
less busted on i686 than it began)
- Fixes some "technically UB" that never had to be UB in the first
place.
- Makes finally follow the noexcept status of the inner function. Maybe
in the future we should ban the function from not being noexcept, but
that is not today.
- Makes various locally-used exceptions inherit from std::exception.
Change-Id: I22e66972602604989b5e494fd940b93e0e6e9297
|
|
Change-Id: I9fbd55a9d50464a56fe11cb42a06a206914150d8
|
|
Change-Id: I16ec8994c6448d70b686a2e4c10f19d4e240750d
|
