Starlark feature request: lazy attr.label/attr.label_list for use with aspects

[dgoldstein0]

Description of the feature request:

Today, bazel assumes that all attributes of a rule are used by the rule's implementation, and eagerly evaluates them at analysis time. This in particular can be expensive for attr.label and attr.label_list attributes, as evaluating these requires analyzing extra targets. What we want is a type of attr.label_list that can be lazily evaluated, whose evaluation could be delayed until access; our desire would be to use it with an aspect.

In particular, we're thinking about giving our libraries services dependencies, as described below. These dependencies are likely to be significantly expensive to build or even just to analyze, and most targets will want to ignore them; but we'd like to be able to apply an aspect, likely via our open source dbx_service_daemon / dbx_service_task rules, which would gather these service dependencies from all the libraries transitively used by the libraries from the wrapped executable. Without lazy evaluation of the service label lists, this is likely to slow down builds of unit tests and individual binaries to an unacceptable level of performance.

What underlying problem are you trying to solve with this feature?

As my former coworker Benjamin Peterson described in his bazelcon 2018 talk (https://www.youtube.com/watch?v=muvU1DYrY0w), Dropbox uses bazel for integration testing - defining processes for the tests to start along with the dependencies between them. We've even open sourced these rules at https://github.com/dropbox/dbx_build_tools

As our codebase has grown, however, the cost of running lots of services for integration testing and development has started to mount. We've been advising internal users for years to make smaller targets with fewer dependencies, but this has not caught on very well:

• dependencies on services can be buried in some other teams code that your code transitively uses
• missing service dependencies cause hard to understand error messages that are not very easily actionable
• adding service dependencies requires an itest-stop/itest-start which can easily take a few minutes

combining these together, the tendency is to start with a large service group target, and whenever some feature doesn't work in that target, add the missing services and forever suffer with an even larger service group with slower builds and slower service startup times - as manually maintaining a more reasonable subset of services to use has proven to be a niche skill.

While we have a ways to go to solve these problems, one belief we have is that if we can treat service dependencies much like regular code dependencies, and have our dbx_js_library, dbx_py_library, and dbx_go_library rules declare their own service dependencies, then we would effectively have outsourced most of these problems to bazel's build graph: authors of the top level service groups could just specify what web pages they want included in the target, or what service, and all the services used by any library they need could just be pulled in.

However, with bazel as is today this comes with a massive cost. for example:

dbx_py_library(
  name = "logging",
  srcs = ["logger.py"],
  services = ["//some/data_collector:service"],
)

now to build any binary including the above library, we at least need to analyze the service dependencies. We expect this to quickly encompass most of our repo for some cases like javascript libraries depending on the services containing their ajax endpoints. Thus we are looking for a way to make these dependencies optional - not just at execution time, but also for analysis, e.g. for (a) just building the libraries or a binary that uses the library and (b) running unit tests which don't want to start any services.

Which operating system are you running Bazel on?

linux

What is the output of bazel info release?

release 6.1.0-0b9dc52a311198b1cde5c2df9403bb5e9529057f

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

We've thought about using config transitions, but those don't seem to solve the basic analysis time blowup of including services as an attribute on our libraries (but to be honest, we're not proficient with config transitions yet - but also some of the issues described in #14023 mean they don't behave as we need, at least by default). We also don't want to build different versions of our libraries with and without services - we just need to propagate extra information up the build graph, which aspects are excellent for.

Any other information, logs, or outputs that you want to share?

No response

[dgoldstein0]

another variation on the idea: provide a way for aspect implementations to turn the strings into Target objects (probably in a batched way). Then we can use attr.string / attr.string_list instead of having an explicit concept of "lazy label".

perhaps a better variation is if the aspect() declaration could have a way to say "I'm going to expand lazy labels" - which could let bazel figure out the lazy labels in the loading phase instead of at analysis - which I suspect is would be easier to implement.

[comius]

Laziness in the attributes is a heavily involved feature request, that has a lot of rough edges.

At the moment there are mechanisms like: aspect_hints attribute that could be analysed only for aspect.
And for the rules there are computed attributes, which means if you don't need a dependency you can compute that before analysis.

Otherwise we're not planning to skip analysis of the attributes if they are not needed by the rule. Why would a rule add them in the first place.

[dgoldstein0]

aspect_hints sounds interesting - first I've heard of it. Problem is, we have multiple aspects that apply to these rules. is there any way to constrain certain aspect hints to only being analyzed for certain aspects? so if a build doesn't apply that aspect, they can skip loading / analyzing the labels contained and all the transitive dependencies of those labels.

for "computed attributes" - what exactly do you have in mind? like select()? or something else?

[comius]

For aspect_hints, what you mention is possible, but not implemented. The attribute can contain arbitrary list of rules. If one was to add required_aspect_hints_providers to an aspect, you could analyze only the rules that declare that provider.

With computed attributes I meant you can set a function to attr.label_list(default = _func), where _func takes some other attributes as a parameters and return a list of labels.

[dgoldstein0]

For aspect_hints, what you mention is possible, but not implemented. The attribute can contain arbitrary list of rules. If one was to add required_aspect_hints_providers to an aspect, you could analyze only the rules that declare that provider.

This sounds interesting. Clarification though - who is "you"? What would work for this use case is to have something like

dbx_js_library(
  name = "foo",
  srcs = [...],
  deps = [":other_js_library", ...],
  aspect_hints_for_services_aspect = [
    "//some:services",
  ]
)

where the //some:services target is expensive to analyze and so I want to incur the cost of that only when an aspect that uses it is applied; i.e. some aspects incur the cost, but others don't. I.e. bzl build //:foo --aspects //:rules.bzl%my_aspect_which_doesnt_use_services would not bother analyzing //some:services given that it doesn't use that particular aspect hint.

I don't think computed attributes help here, but it is interesting to know about. Would be curious if the input to the default function are specified somewhere.

[brandjon]

A few months ago I might've dismissed this suggestion as too much of a change for a non-critical use case. Now I'm not so sure. What's changed is that we now intend to prototype lazy load()ing in the near future, based on a change to Skyframe's that allows Starlark evaluation to resume after requesting additional dependencies without restarting from the beginning of the Starlark code. I could imagine the same capability being used to implement lazily-requested label dependencies.

There might be other constraints about rule analysis (e.g. related to configuration machinery?) that come into play. And it may have other API/usability concerns that I haven't thought of (because I've given this all of 10 seconds of thought so far). But it's not as technically infeasible as it used to be.

[brandjon]

(Assigning team-Rules-API but this could also be team-Starlark-Integration.)

[dgoldstein0]

@brandjon so an alternative idea that @comius suggested was that this use case could also be satisfied by some extension of aspect_hints - we'd need hints that are (a) specific to a subset of aspects and (b) only get analyzed if that aspect is applied to the target. Would that be more likely to be implemented sooner than the lazy attributes idea I pitched here?

[github-actions]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

[dgoldstein0]

This feature would still be super useful for my company. Basically we need some way to only pay the analysis cost of a label_list for an aspect that uses it, and not for usages of the rule that don't apply the aspect. That could be lazy attributes or some sort of aspect_hints field that is evaluated just for our aspect.

[fmeum]

Did you see https://docs.google.com/document/d/1IqzjxpB9jnJMvkYCgwawAdvXkdjenEDeCxu1S-D-XQk/edit, which might be related?

[dgoldstein0]

I have not yet. Thanks for sharing, it does sound related, I'll read it this week.

…

On Tue, Sep 17, 2024, 8:18 AM Fabian Meumertzheim ***@***.***> wrote: Did you see https://docs.google.com/document/d/1IqzjxpB9jnJMvkYCgwawAdvXkdjenEDeCxu1S-D-XQk/edit, which might be related? — Reply to this email directly, view it on GitHub <#17948 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIGO53N73A3DCH5P3LJLDLZXBB25AVCNFSM6AAAAABOKNQYUOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJWGIYDQNRTGY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[dgoldstein0]

I read the doc and asked a few questions. It certainly feels like it's in a similar problem space, though it's completely unclear how it interacts with aspects. One of our desires is to not have to edit every rule we use to propagate our own providers, and unless we can somehow use those DormantDependency things with aspects, we're unlikely to be obviously better off with than just using our current "wrap every rule in a macro and have that macro declare targets for a parallel dependency graph" approach (which has some very sharp edges).

It's worth noting that proposal is motivated somewhat backwards of what we're doing - they propose that it's common for test builds to need fewer dependencies than prod builds due to dependency injection, but our integration testing setup is the exact opposite - in prod we use service discovery & grpc calls to talk to other services, whereas in integration tests we build all the other services into our test targets, have a small orchestrator that starts all the services, and then call out to the running copy of the service that was included in our target. That distinction, however, doesn't seem like it should stop us from using dormant dependencies.

[fmeum]

Cc @lberki

[lberki]

The motivation was not as much as "tests have fewer dependencies than prod", more like "tests have different dependencies than prod". From a quick skim of your problem statements, dormant deps are exactly what you need, albeit they'd need some tweaking to be really useful to you.

The way I imagine they could help is that your binary rule would propagate an aspect to all the libraries in its transitive closure to gather all the services they think they need, then that aspect would materialize only the services that are actually needed.

However, this doesn't quite work with the current implementation because data from aspects cannot currently be used to make materialization decisions (everything that can affect such a decision must by marked as dependency_resolution_rule and aspects cannot be so marked)., This is not an inherent limitation, it's just that the use case we implemented it for doesn't require aspects and I found it easier just to blanket prohibit them so that it's easier to think about the problem space.

I could imagine propagating an aspect and the aspect materializing the services instead. This has the downside that then the decision must be "local", i.e. if binary A depends on libraries X and Y separately, the decision whether to materialize a service X declares cannot depend on what services Y declares. It's also impossible with the current implementation, since aspects currently cannot have materializers. It's not an inherent limitation, either, just an artificial one to make it easier for me to prove the initial implementation correct.

[dgoldstein0]

I don't think the materialization with aspects will be an issue. for every test with services, we're actually having our macros generate a dbx_service_test that then uses the test - so e.g. our python tests with no services are just our normal python test rule, but the ones with services are a dbx_service_test(exe=":python_test_target") roughly. which means dbx_service_test could apply the aspect , which would gather the dormant dependencies, and do the materialization. The only challenge is if we want some logic as to which dep to materialize, that could potentially get gnarly but as a first pass I don't think we'll need anything fancy.

I suppose we will have some annoyance in needing to add new attributes to each type of library rule we're trying to support, but that's certainly a lot better than our current approach.

[lberki]

This approach wouldn't work with the implementation at HEAD because you (currently) can't propagate an aspect and feed its providers into a materializer.

There is no fundamental reason why that has to be so, it's just that we wanted to segregate rules/aspects whose logic can affect the dependency graph and rules/aspects whose logic cannot, and the simplest thing was to simply prohibit aspects entirely. Allowing aspects would work, at the cost of some thinking on my side as to how to enforce the "dependency resolution things cannot depend on non-dependency-resolution things" invariant.

[dgoldstein0]

understood. I don't really understand why that would be significantly more complex to implement, but understand if it's not in the MVP of DormantDependency.

Is dormant dependencies definitely happening? if so I can rewrite this ticket as an ask for supporting materializing providers propagated from aspects with dormant dependencies.

[lberki]

It's definitely happening, it's already available at HEAD behind --experimental_dormant_deps.

It's that aspects can depend on other aspects, rules can propagate aspects, aspects can propagate aspects, etc., so it's more moving parts, all of which cases needs to be considered.