dart-lang / language

Design of the Dart language
Other
2.65k stars 202 forks source link

A more intentional and query-like API for macro source introspection #3706

Open lrhn opened 5 months ago

lrhn commented 5 months ago

The current API for introspecting the source of the code that the macro is generating augmentations for, is fairly coarse-grained.

The example requests all methods from a class, then manually goes through them to check if there is a method named toString. That hides the actual data used by the macro from the macro runtime, it cannot see which methods are actually needed, and which properties of the methods are needed.

I suggest instead having an API where you'd specifically ask for "instance methods named toString", and get only the zero or one result, and the object returned would only contain an ID, the name, and that it's an instance method. It wouldn't contain parameter or return types, if you don't ask for those. Or there might even be a function to just answer whether a match exists, returning just a bool.

The request would be sent as a query object to the macro runtime.

(That behaves something like an SQL query of SELECT id, name, static FROM members WHERE containingClassId = classId, name = "toString", kind = "method", static = false, meaning that you don't necessarily get fully populated objects. You'd have to be able to ask whether a property is available, so you can do if (method.hasStatic) to check if the "is this method static" boolean is even available, and it'll throw if you do mathod.isStatic and the information isn't there. You're expected to have asked for the information you need.)

The potential benefit of using such an API is that the query objects are reusable and replayable, and the runtime system can see precisely which information the macro code has requested, and presumably depends on. Also, the query can be much more targeted, and won't provide results that the macro won't be able to understand, because any new kind of declaration would have to be requested. There is no "give me everything" request that can give you results of types that you don't know about.

If the program changes, like dynamic reload or just during development, the runtime system can potentially check whether any answer it would give to a macro has also changed. If not, there is no need to re-run the macro, since it would have precisely the same behavior the second time. (That requires some way to keep IDs consistent across changes.)

A possible API could be:

  ClassDeclaration clazz = ...;
  if (builder.queryAny(clazz.members(MethodFilter(name: "toString", static: false)))) {
    // There is a non-static method named `toString`
    ...
  }

The query methods could be something like:

  T? queryFirst<T extends SourceEntity>(Query<T> query);
  List<T> queryAll<T extends SourceEntity>(Query<T> query);  
  bool queryAny(Query<SourceEntity> query);

and the way to query members of a class is:

Query<T> members<T extends ClassMember>(MemberFilter<T> filter) => filter._query

with a

class MemberFilter<T extends MemberDeclaration> extends DeclarationFilter<T> implements Filter<T> {
  BoolFilter static = BoolFilter.ignore; // Not queried as default.
  // ...
}
class MethodFilter<T extends MethodDeclaration> extends MemberFilter<T> {
  // method-specific properties.

  // Accepts
  MethodQuery<T> _queryForContainer(ContainerDeclarationID id) => MethodQuery<T>(
    containingDeclaration: id,
    name: this.name,
    static: this.static,
    // ...
  );
}

and a returned MethodDeclaration could be:

abstract class MethodDeclaration extends MemberDeclaration {
  MethodDeclarationId? get id;

  // Inherits `name`, `static` and `containingDeclarationId`

  bool get hasReturnType;
  // Throws if [hasReturnType] is false.
  TypeReference get returnType;

  bool get hasParameters;
  ParameterListDeclaration get parameters;
}

Or something like that. The point is to:

That means that there is no query to get "all members". The query would have to ask for MemberFilter(kinds: [MethodDeclaration.kind, SetterDeclaration.kind, GetterDeclaration.kind, ConstructorDeclaration.kind, VariableDeclaration.kind]); to get all the declarations. If we introduce nested type aliases, you wouldn't start getting them until you add TypeAliasDeclaration.kind to the list.

Or maybe we can combine filters, so MethodFilter(static: BoolFilter.no) | GetterFilter(static: BoolFilter.no) | SetterFilter(static: BoolFilter.no) would query all non-static, non-constructor members.

(We can also have subclasses named StaticMethod and InstanceMethod, which hard-code the static query.)

If we're really clever, we may be able to see that phase 1 and 2 of a macro doesn't need to be run again, but that requires caching the results after those phases, to be able to run phase 3 on top of that. It may still imply that the type signature of the generated code has not changed, and so hot-reload may be able to determine that other libraries do not need to be rerun, even if we are going to run the macro again. (With a fallback if phase 1/2 actually does something different, which it shouldn't since macros should be deterministic.)

jakemac53 commented 5 months ago

I do think we should do something along these lines, as well as looking at:

davidmorgan commented 4 months ago

I have been prototyping in this area and should have some results to share soon.

davidmorgan commented 4 months ago

https://github.com/dart-lang/language/tree/main/working/macros/dart_model now has some runnable macros using a query approach, plus a guide to setting up a huge example to see how it fares from a performance point of view.

davidmorgan commented 4 months ago

Some notes on that huge example performance here https://github.com/dart-lang/sdk/issues/55784#issuecomment-2141636339

davidmorgan commented 4 months ago

Here is a high level description of what the exploratory code does right now:

On startup a macro sends a Query to say what it wants for its input and gets in response a Stream<Delta>. When a Delta arrives it applies this to its local Model, regenerates augmentations based on it, and sends them.

The macro host keeps a map from Query objects to macros; when files change it reruns the queries, sends out the deltas, waits for augmentations from all the macros in response, merges them then writes them. Then it reanalyzes, computes deltas again, and reruns until there are no changes.

Probably the most useful code to look at is a macro and the macro host; you can try both interactively via the example in the README.

jakemac53 commented 4 months ago

Forking from the discussion started in https://github.com/dart-lang/sdk/issues/55784 - I want to be able to answer the question "what is it about this approach which makes it fundamentally better than the existing API/proposal".

There are many different concepts which are heavily intertwined here - and the current experimental code works in fundamentally different ways, bearing almost no relation at all to current macros feature. Each of these differences has their own impacts on performance, some of which may not be viable for a shipped product here and others which may be.

So, I want to identify exactly what the impactful changes are, why they are impactful, whether they are shippable as a language feature, whether they can simply be ported to the existing proposal, need a whole new proposal, etc.

I wrote down my really quick thoughts below, please correct anything you think is wrong:

Query-like API

I understand that by having a more fine grained query based API, we could have better invalidation. I do not however believe this is a meaningful contributor to the performance differences that have been advertised. More specifically, I think library level invalidation would provide a similar level of performance benefit (not as much, but enough to solve the large library cycle problem).

In other words, I think this is a red herring. To be clear though, I do think we should move forward with it, because I think it is a better API and will help performance, its just not the primary driving factor here.

Pure string based code generation

I actually believe this is the primary contributor to the performance benefits as it stands today, however crazy that sounds.

If all "identifiers" in code were actually resolved as a part of producing the code (they are in the current feature), then unless you jump through a lot of hoops you end up needing to invalidate a ton of macro applications (or at least the part that produces the augmentation file), whenever any macro changes. This is the primary revelation that came about from the discussions in https://github.com/dart-lang/sdk/issues/55784.

tldr; In effect, you end up today with an implicit dependency on all libraries you import, and whenever their macros are re-ran yours also need to be re-ran. This is all just because of the potential for shadowing, and in general top level identifiers (like types) being introduced by those libraries.

In the prototype you have however, you are not relying on the host to do any identifier lookup. The macros just refer to things by name, assume they are in scope, etc. My proposed fix in the linked issue is to actually do much the same as this, to avoid this implicit dependency on where identifiers actually come from.

No phases

This may also be a large contributor, I am not quite sure yet exactly, but it is possible that it helps some.

Delta based generation

While potentially nice, I also don't believe this is a significant contributor, on the order of magnitude of the speed increase being shown in the prototype.

Even if we did a "rounds" based approach instead of phases, I think it would be fine if each macro just re-ran from scratch in each round. Or at least, that wouldn't be so bad for performance as to tank the feature as a whole.

davidmorgan commented 4 months ago

Thanks Jake! Some thoughts.

Query-like API.

It's possible that the better invalidation / smaller data size doesn't bring much; but it does allow us to push as close to the theoretical best as we want to, so if there are program structures that turn out to be important that we want to be faster, we can refine to make that happen. (Unless they fundamentally require too much work). I think this will be easier to maintain in the long run and through language changes than the dependency-tracking approach.

The resulting capability to "batch" requests and responses ... combining most macro requests into one request, then giving one response ... is probably more important for performance. #3873 talks about avoiding duplicate work across applications, for the query-like API this means you query once rather than querying multiple times for the same data.

String-based code generation.

This is certainly an important issue; it's sidestepped in the "trivial macros" benchmarks because they are trivial enough to not output any types at all :) ... in other benchmarks I've looked at, you are correct, the types are just assumed.

Agreed, this looks like a good direction independent of the query-based API. Where it ties back to the query-based API is the concept of writing everything on the wire, i.e. requiring that identifiers be sufficiently self describing rather than allowing the host to use an opaque ID that it tracks locally. "Unpacking" that opaque ID into data is needed to get all the nice properties of the data model. So the implementation we end up with can be different depending on how full-featured we want the data model to be.

No phases.

Not sure :) hoping to dig on this one on #3868 ... I expect we can pull as much as we like of the phases into the query-like API model without too much trouble, so whatever is useful in terms of performance and/or correctness.

Deltas.

The logic of the macros is usually very simple so the runtime is pretty much negligible ... so across the various benchmarks so far I didn't really bother to avoid redoing computation. But deltas also save on the volume of data sent, and I think that part is significant.

The delta from someone typing should always be small :) so if we are sending deltas we should always be able to communicate the change quickly to a macro, which can rerun from scratch if it likes; and then if we want to we can worry about the size of the response and maybe do something similar there.

jakemac53 commented 4 months ago

"Unpacking" that opaque ID into data is needed to get all the nice properties of the data model. So the implementation we end up with can be different depending on how full-featured we want the data model to be.

This is the main part about the data model that concerns me, we are potentially talking about a very large data size increase. I don't believe it is necessary either. We should be able to compute the hash of a query result based on the full data model, but still only send IDs for things that we know are cached client side? In other words the hash for the query result, wouldn't be based on the actual data sent on the wire, but the hashCode implementation of the instances. This would importantly not include the IDs.

The logic of the macros is usually very simple so the runtime is pretty much negligible ... so across the various benchmarks so far I didn't really bother to avoid redoing computation. But deltas also save on the volume of data sent, and I think that part is significant.

Ah yeah that could be interesting to only send deltas for the queries, with the cached results on the macro side of things? That does get more interesting if we are also sending IDs, those IDs and the cache itself are going to be much longer lived, which could be good or bad.

davidmorgan commented 4 months ago

Yes, data size is a concern :) fortunately I think it can be solved in an even nicer way, messages can be self contained, but with long strings deduplicated: you write the full string once then refer to it within the message by ID; when you deserialize you replace the IDs with references to the same string instance. Then there is no reliance on local state. You do still have to mention every ID in full once per message, but that shouldn't be too bad.

Right, it should be possible to send deltas for query results with a cache on the client; of course we can also choose to resend the whole thing if we want to save memory in the macro, or if it has to restart.