How does the rule parsing and scheduler initialization work Pants #plugins

How does the rule-parsing and scheduler initializa...

ambitious-actor-36781

04/12/2023, 3:24 AM

How does the rule-parsing and scheduler initialization work? I've got some rules that seem to spike the startup of pants from ~3s to 30s And commenting out the rules speeds it up, but it seems like it's a combo of rules that is making it a problem? Think I've found it. Maybe? It looks like wrapping a rule's return from

AThing -> Digest

to look more like

AThing -> Wrapper(Digest)

fixes the problem.

curved-television-6568

04/12/2023, 9:01 AM

Ah, yes it’s likely a good idea to have unique-ish result types for your rules. Otherwise you may get unwanted results as there may be other rules that return a

Digest

for instance which may be used instead of the rule you intended.

ambitious-actor-36781

04/12/2023, 9:09 AM

I had assumed it was down to a mapping of

request -> response

ambitious-actor-36781

04/12/2023, 9:14 AM

but if it's return types, that's an easy enough change

curved-television-6568

04/12/2023, 9:18 AM

It is true, but you may have

request -> foo -> bar -> baz -> response

request -> some -> thing -> else -> response

.. 🤷 😉

ambitious-actor-36781

04/12/2023, 9:19 AM

the engine traverses rules?

curved-television-6568

04/12/2023, 9:19 AM

indeed it does, you ask for

given

and inspects every possible path through all known rules to satisfy that

ambitious-actor-36781

04/12/2023, 9:20 AM

that's uh. kinda dope and also... seems unnecessary?

curved-television-6568

04/12/2023, 9:21 AM

dope yes, unnecessary no. That hides a lot of complexity for you. And makes you sane while implementing rules 😉 as you get to focus on just one small step at a time

curved-television-6568

04/12/2023, 9:22 AM

without it, it wouldn’t be much more than just calling the rules directly. The power lies in that you don’t have to figure out all the inbetween rules to call to get what you want.

ambitious-actor-36781

04/12/2023, 9:23 AM

well damn. I've just been treating it as some kind of dependency-injection/dispatching system.

ambitious-actor-36781

04/12/2023, 9:24 AM

just to confirm. if I had some rules that did:

Target -> IntermediateA

IntermediateA -> IntermediateB

and

IntermediateB -> Digest

I could just ask it

Get(Digest, Target)

👍 1

curved-television-6568

04/12/2023, 9:25 AM

consider the rules you’ve been using all along, they certainly are chained more than just a few levels already (like getting hydrated sources with code gen etc)

ambitious-actor-36781

04/12/2023, 9:26 AM

Time to rewrite all my plugins.

curved-television-6568

04/12/2023, 9:26 AM

sorry about that 🤣

curved-television-6568

04/12/2023, 9:29 AM

but, I would guess you’d not be the only nor the first one to think like this, so we should probably make some doc effort to highlight this better.. 🤔 cc @happy-kitchen-89482

👍 1

witty-crayon-22786

04/12/2023, 4:42 PM

it’s a fairly expensive feature (at graph solving time, but not runtime) that is used in a variety of places, but which is very non-obvious.

👀 1

witty-crayon-22786

04/12/2023, 4:42 PM

i’ve failed to come up with a more obvious syntax for it though (which hopefully would make graph solving cheaper as well)

curved-television-6568

04/12/2023, 5:01 PM

I don’t see how the syntax could affect this.. or?

witty-crayon-22786

04/12/2023, 7:10 PM

So, currently: there is no syntax... every callsite might implicitly involve a conversion/overloading. The only place where that isn't the case is with rule_helpers / async defs which are called directly.

witty-crayon-22786

04/12/2023, 7:19 PM

But you could imagine having a syntax that explicitly opted you in to the solving... or out.

👍 1

witty-crayon-22786

04/12/2023, 7:21 PM

One example would be allowing for directly referencing the name of the rule that you're expecting to call

witty-crayon-22786

04/13/2023, 4:32 PM

i want to continue brainstorming this, because it’s been on my mind for a while.

witty-crayon-22786

04/13/2023, 4:33 PM

one strawexample would be something like

Copy code

await Call(my_rule_function, AnArgument(..))

witty-crayon-22786

04/13/2023, 4:33 PM

(you can’t directly use call syntax because the point is that we’re dependency injecting an arbitrary number of additional arguments)

witty-crayon-22786

04/13/2023, 4:34 PM

…oh. OR. we could theoretically have the

@rule

decorator transform calls into shims that would capture their arguments…

curved-television-6568

04/13/2023, 4:36 PM

I like the shim approach. That keeps the magic out of sight so it looks like just another call

witty-crayon-22786

04/13/2023, 4:39 PM

some amount of obvious magic might still be good, so that it’s obvious that something is happening… maybe:

Copy code

await my_rule_function(AnArgument(..), ...)

witty-crayon-22786

04/13/2023, 4:39 PM

Copy code

>>> def callee(*args, **kwargs):
...   print(args)

…

Copy code

>>> callee("one", ...)
('one', Ellipsis)

witty-crayon-22786

04/13/2023, 4:41 PM

an annoying downside though is that currently rule names don’t matter at all… suddenly, both types and rule names would matter in public APIs

curved-television-6568

04/13/2023, 4:45 PM

Copy code

await my_rule_function(AnArgument(..), ...)

this doesn’t look like magic to me, but I might have not used await if the rule wasn’t declared async..

curved-television-6568

04/13/2023, 4:46 PM

maybe have a new concept of “rule id” that you provide to the

@rule

decorator and only rules with a specified id may be called this way?

witty-crayon-22786

04/13/2023, 4:52 PM

to be clear, i mean literally requiring the ellipses. but yea, not too bad

👍 1

curved-television-6568

04/13/2023, 4:53 PM

that’s a good guard rail to protect against mistakes about thinking it actually is a regular call

witty-crayon-22786

05/03/2023, 11:46 PM

thought more about this today, and filed https://github.com/pantsbuild/pants/issues/18895

💯 1

2 Views

Open in Slack

Previous Next