More performance improvements to try

[davidmorgan]

Hi everyone! Following on from this discussion, another chance to try improvements before they're released.

As before you can try unreleased versions with overrides in your pubspec.yaml then dart pub get:

dependency_overrides:
  build:
    git:
      url: https://github.com/dart-lang/build
      path: build
      ref: # put a ref here
  build_config:
    git:
      url: https://github.com/dart-lang/build
      path: build_config
      ref: # put a ref here
  build_modules:
    git:
      url: https://github.com/dart-lang/build
      path: build_modules
      ref: # put a ref here
  build_resolvers:
    git:
      url: https://github.com/dart-lang/build
      path: build_resolvers
      ref: # put a ref here
  build_runner:
    git:
      url: https://github.com/dart-lang/build
      path: build_runner
      ref: # put a ref here
  build_runner_core:
    git:
      url: https://github.com/dart-lang/build
      path: build_runner_core
      ref: # put a ref here
  build_test:
    git:
      url: https://github.com/dart-lang/build
      path: build_test
      ref: # put a ref here
  build_web_compilers:
    git:
      url: https://github.com/dart-lang/build
      path: build_web_compilers
      ref: # put a ref here

I'll add new posts with a ref when there is something interesting to try.

[davidmorgan]

At ref 0551fa2 is the new "triggering" heuristic, to cut down on time spent doing nothing when a builder doesn't apply.

It does nothing out of the box, to make it do something add to your own build.yaml something like:

targets:
  $default:
    builders:
      built_value_generator:built_value:
        options:
          run_only_if_triggered: true

triggers:
  built_value_generator:built_value:
    - import built_value/built_value.dart
    - import built_value/serializer.dart

--specifying which direct imports trigger each builder you're using. (For some builders this can probably end up directly in the builder config so you don't have to do anything!)

If the heuristic does something you'll see output with "not triggered", like

2s built_value_generator:built_value on 1001 inputs: 999 not triggered, 2 output; spent 1s analyzing                                                                                                                                                         
2s source_gen:combining_builder on 1001 inputs: 2 output, 999 no-op; spent 1s building 

which means that build step was not triggered and so did nothing ... quickly :)

This is the first change after the element2 release build 3.0.0, I know not all builders are released for this version yet. If you're blocked because of that please let me know, I will see if I can help out!

[josh-audio]

This looks great! I just upgraded my only external builder dependency to build: 3.0.0 and I'm about to upgrade my internal builders, so I will give this a try once that work is done.

A couple questions:

• Is this appropriate (or even possible) for library authors to use? If a library author were to add a trigger to their build.yaml, would downstream consumers inherit the triggers list, or is this always local to the current package?
• Is this just a text search, or is there a deeper analysis going on? E.g., if I were to export library A from library B, and if my triggers list has only library A, would an import of B count as a matching trigger?

[davidmorgan]

• Is this appropriate (or even possible) for library authors to use?

That's the intention, yes.

If a library author were to add a trigger to their build.yaml, would downstream consumers inherit the triggers list, or is this always local to the current package?

Triggers lists are collected from all packages in your codebase, so they can come from the builder, from your own settings, or from any package you use.

• Is this just a text search, or is there a deeper analysis going on? E.g., if I were to export library A from library B, and if my triggers list has only library A, would an import of B count as a matching trigger?

Just a text search.

Library B should add a trigger in its own build.yaml to keep things working for everyone.

Thanks :)

[josh-audio]

Makes sense, thanks for the information.

Talking about "library", I meant from a language perspective. I'm sure someone out there has something like:

// codegen_annotations.dart

// Re-export all codegen annotations for convenience

import 'package:my_codegen_lib/annotations.dart';
// More imports here...

export 'package:my_codegen_lib/annotations.dart';
// More exports here...

// This file expects to run code generators from my_codegen_lib

import 'path/to/codegen_annotations.dart';

@MyAnnotation
class MyClass {
  // ...
}

If my_codegen_lib adds a trigger for import 'package:my_codegen_lib/annotations.dart', then this project would quit receiving code generation. This might be reasonable, but if I were a library author, I would feel uneasy about this.

As an alternative, would it be possible to instead use @MyAnnotation as a trigger, or do the triggers only work with imports? I know this doesn't work for all code generators, but I'm thinking about this in the context of MobX which uses annotations as the only trigger for codegen, and I think it would be far less likely for a given downstream package to alias an annotation than to re-export an import.

[davidmorgan]

If my_codegen_lib adds a trigger for import 'package:my_codegen_lib/annotations.dart', then this project would quit receiving code generation. This might be reasonable, but if I were a library author, I would feel uneasy about this.

Yes, this needs to be good enough. It's not clear yet precisely what that means.

The build_runner output will say "not triggered" for those cases, which is a bit of a clue, but probably not enough :)

As an alternative, would it be possible to instead use @MyAnnotation as a trigger, or do the triggers only work with imports? I know this doesn't work for all code generators, but I'm thinking about this in the context of MobX which uses annotations as the only trigger for codegen, and I think it would be far less likely for a given downstream package to alias an annotation than to re-export an import.

Yes, we can do anything that's fast: matching strings, even parsing would be a lot faster than resolving. For example "uses annotation named ...", "uses type named ...", "has (top level) variable/function/method/class named ...".

[josh-audio]

The build_runner output will say "not triggered" for those cases, which is a bit of a clue, but probably not enough :)

Agreed that this is likely not enough - acting on this requires having a mental model about build_runner that I personally didn't have until I started writing my own builders. Though from a diagnostic standpoint it's still very helpful to have that message!

Yes, we can do anything that's fast: matching strings, even parsing would be a lot faster than resolving. For example "uses annotation named ...", "uses type named ...", "has (top level) variable/function/method/class named ...".

Fair enough - it makes sense that resolving would defeat the purpose, but operating on a per-file analysis is an interesting route. With the right options, I'm sure most library authors will be able to feel confident implementing this.

I'll keep watching this, thanks again for the information! Looking forward to testing it out soon.

[rrousselGit]

Is the lint being worked on? :)

I personally woudln't ship this unless the lint to spot mistakes was also shipped.
It's too error prone when we consider exports/const variables/typedefs/extension types/...

[davidmorgan]

Is the lint being worked on? :)

I personally woudln't ship this unless the lint to spot mistakes was also shipped. It's too error prone when we consider exports/const variables/typedefs/extension types/...

Not yet; since it's possible to use with just local config, I'll look for first feedback from that first, then see if we can apply it to some builders (ones that are not usually triggered in complex ways) then think about the lint.

[josh-audio]

Just tried it out. My main build.yaml looks like this:

targets:
  $default:
    builders:
      anthem_codegen|anthemDartModelGeneratorBuilder:
        generate_for:
          - lib/*
          - lib/**/*
        options:
          run_only_if_triggered: true
      anthem_codegen|cppModelBuilder:
        generate_for:
          - lib/*
          - lib/**/*
        options:
          run_only_if_triggered: true
      anthem_codegen|debugEnginePathGeneratorBuilder:
        generate_for:
          - lib/engine_api/engine_connector.dart
      mobx_codegen|mobx_generator:
        options:
          run_only_if_triggered: true

triggers:
  anthem_codegen|anthemDartModelGeneratorBuilder:
    - import anthem_codegen/include/annotations.dart
  anthem_codegen|cppModelBuilder:
    - import anthem_codegen/include/annotations.dart
  mobx_codegen|mobx_generator:
    - import mobx/mobx.dart

Before:

18s mobx_codegen:mobx_generator on 195 inputs: 33 same, 162 no-op; spent 15s analyzing, 3s resolving
1s anthem_codegen:anthemDartModelGeneratorBuilder on 187 inputs: 27 same, 160 no-op
1s source_gen:combining_builder on 195 inputs: 34 same, 161 no-op
1s mockito:mockBuilder on 8 inputs: 4 skipped, 1 same, 3 no-op
0s anthem_codegen:debugEnginePathGeneratorBuilder on 1 input: 1 same
2s anthem_codegen:cppModelBuilder on 375 inputs: 153 skipped, 28 same, 194 no-op; spent 2s analyzing

Built with build_runner in 25s; wrote 150 outputs.

After:

0s mobx_codegen:mobx_generator on 195 inputs: 195 not triggered
0s anthem_codegen:anthemDartModelGeneratorBuilder on 187 inputs: 187 not triggered
0s source_gen:combining_builder on 195 inputs: 195 no-op
16s mockito:mockBuilder on 8 inputs: 4 skipped, 1 same, 3 no-op; spent 12s analyzing, 3s resolving
0s anthem_codegen:debugEnginePathGeneratorBuilder on 1 input: 1 same
0s anthem_codegen:cppModelBuilder on 375 inputs: 187 skipped, 188 not triggered

Built with build_runner in 18s; wrote 2 outputs.

The complete no-op is surprising to me. One of the files that I expect to run codegen has the following imports:

import 'package:anthem/helpers/id.dart';
import 'package:anthem/model/anthem_model_base_mixin.dart';
import 'package:anthem_codegen/include/annotations.dart';
import 'package:mobx/mobx.dart';

Is there anything additional I could provide to help diagnose the issue?

[josh-audio]

Thanks! I thought I tried that, but evidently not. Didn't realize that syntax was deprecated - will update my configs.

Here's the output after adding a comment to one of my source files, in case you're curious.

17s mobx_codegen:mobx_generator on 195 inputs: 155 skipped, 31 same, 9 no-op; spent 13s analyzing, 2s resolving
0s anthem_codegen:anthemDartModelGeneratorBuilder on 187 inputs: 160 skipped, 26 same, 1 no-op
1s source_gen:combining_builder on 195 inputs: 164 skipped, 31 same
2s mockito:mockBuilder on 8 inputs: 5 skipped, 1 same, 2 no-op; spent 2s analyzing
0s anthem_codegen:debugEnginePathGeneratorBuilder on 1 input: 1 skipped
1s anthem_codegen:cppModelBuilder on 375 inputs: 347 skipped, 28 same

Built with build_runner in 23s; wrote 143 outputs.

The MobX import trigger is crude in this case, because it's a catch-all import. The MobX import is used in a few files with no codegen, as you can see above. Seems like it wouldn't make much practical difference in my case though.

[davidmorgan]

Thanks!

Is that from build or watch?

If it's build, I wonder how much of that "analyzing" time would be saved in watch mode; the main performance difference in watch mode is that the analyzer keeps its state, so any analysis results that can be retained will be retained.

[josh-audio]

This is build.

I don't use watch mode in this project because I'm generating C++ code. Whenever I run a build_runner build, it touches every possible output file in a way that makes both MSVC and Clang think that the files have changed, even if they haven't. Currently this costs me somewhere around 2 minutes of extra C++ build time, though I expect this will grow.

I mitigate this with a wrapper script that copies the old C++ files out before a build, and then puts old files back if the content is the same as the new file. At least half my Dart rebuilds result in no C++ change, so this saves me a lot of time compared with the extra ~12s analyze cost. I don't use watch mode because I'm not aware of any way to hook this script into the lifecycle for watch.

I should say that I haven't actually tried to see if watch mode has this issue, so I'll do that when I get the chance. I've been assuming it's the same process as build, just with some caching.

[davidmorgan]

I don't use watch mode in this project because I'm generating C++ code. Whenever I run a build_runner build, it touches every possible output file in a way that makes both MSVC and Clang think that the files have changed, even if they haven't. Currently this costs me somewhere around 2 minutes of extra C++ build time, though I expect this will grow.

Oh, interesting. That sounds well worth fixing. Filed #4113

I would guess it also happens in watch mode.

[josh-audio]

Much appreciated! Didn't think to file an issue since it doesn't affect Dart. I'll be watching that one for sure.

[davidmorgan]

FYI: I'll be on vacation next week, I think I'll wait until I'm back to release anything; so: early August.