Skip to content

consider benchmarking the lints #57574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
devoncarew opened this issue May 16, 2017 · 8 comments
Closed

consider benchmarking the lints #57574

devoncarew opened this issue May 16, 2017 · 8 comments
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. type-performance Issue relates to performance or code size

Comments

@devoncarew
Copy link
Member

We've had issues where individual lints can have outsized performance impact on the time for analysis. We should set up a benchmarking system, so we can get a rough sense of which lints are performant, slow, or have severe perf issues.

@pq

I have worked on some similar systems in the past - we may be able to share some code or concepts here.

@devoncarew devoncarew changed the title consider benchmarking each lints consider benchmarking the lints May 16, 2017
@bwilkerson
Copy link
Member

The linter has a --stats flag that will provide timing information for each of the lints run over whatever code base is being linted. That should be a reasonable start for automating something.

@pq
Copy link
Member

pq commented May 17, 2017

Sounds great.

As for the stats flag, Brian is right it gives us a nice jumping off point.

For context, here are the salient details running all the lints against the linter codebase:

---------------------------------------------------
Timings                                          ms
---------------------------------------------------
always_declare_return_types                      18
always_put_control_body_on_new_line              15
always_require_non_null_named_parameters         24
always_specify_types                             19
annotate_overrides                               31
avoid_annotating_with_dynamic                     8
avoid_as                                          8
avoid_catches_without_on_clauses                  8
avoid_catching_errors                             8
avoid_classes_with_only_static_members           10
avoid_empty_else                                  8
avoid_function_literals_in_foreach_calls          9
avoid_init_to_null                                9
avoid_null_checks_in_equality_operators          10
avoid_positional_boolean_parameters              25
avoid_return_types_on_setters                     8
avoid_returning_null                             25
avoid_returning_this                             10
avoid_setters_without_getters                    11
avoid_slow_async_io                              11
avoid_types_on_closure_parameters                10
await_only_futures                                7
camel_case_types                                 12
cancel_subscriptions                             24
cascade_invocations                              18
close_sinks                                      34
comment_references                                8
constant_identifier_names                        10
control_flow_in_finally                          10
directives_ordering                              26
empty_catches                                     7
empty_constructor_bodies                          7
empty_statements                                  6
hash_and_equals                                   8
implementation_imports                            8
invariant_booleans                               88
iterable_contains_unrelated_type                 11
join_return_with_assignment                      11
library_names                                     9
library_prefixes                                  8
list_remove_unrelated_type                        8
literal_only_boolean_expressions                  9
no_adjacent_strings_in_list                       7
no_duplicate_case_values                         11
non_constant_identifier_names                    19
omit_local_variable_types                        12
one_member_abstracts                              8
only_throw_errors                                 8
overridden_fields                                14
package_api_docs                                 27
package_prefixed_library_names                    8
parameter_assignments                            12
prefer_adjacent_string_concatenation              8
prefer_collection_literals                        7
prefer_conditional_assignment                     8
prefer_const_constructors                         8
prefer_constructors_over_static_methods           7
prefer_contains                                  15
prefer_expression_function_bodies                 9
prefer_final_fields                              16
prefer_final_locals                              13
prefer_foreach                                    8
prefer_function_declarations_over_variables       8
prefer_initializing_formals                      17
prefer_interpolation_to_compose_strings          11
prefer_is_empty                                  17
prefer_is_not_empty                              14
public_member_api_docs                          117
recursive_getters                                11
slash_for_doc_comments                           10
sort_constructors_first                          12
sort_unnamed_constructors_first                   9
super_goes_last                                   8
test_types_in_equals                              8
throw_in_finally                                  7
type_annotate_public_apis                        12
type_init_formals                                 6
unawaited_futures                                 9
unnecessary_brace_in_string_interps               8
unnecessary_getters_setters                      10
unnecessary_lambdas                              11
unnecessary_null_aware_assignments                7
unnecessary_null_in_if_null_operators             7
unnecessary_overrides                            18
unnecessary_this                                 33
unrelated_type_equality_checks                   11
use_rethrow_when_possible                         7
use_setters_to_change_properties                 11
use_string_buffers                               10
use_to_and_as_if_applicable                      13
valid_regexps                                     8

@devoncarew : let's set aside some time later this week to brainstorm.

Looking forward! 👍

@devoncarew
Copy link
Member Author

Ah, fantastic! I didn't realize. Seems like much of the work is done already.

@pq
Copy link
Member

pq commented May 19, 2017

Picking up from dart-archive/linter#671, the next steps are to:

  1. emphasize timings (over counts) in output
  2. do multiple runs to improve the value of the stats

To that end, I'd like to consider a new flag (-b or --benchmark) that runs the linter 10 times, picks the best results (a la golem) and displays just the timings.

@devoncarew @bwilkerson for thoughts.

@devoncarew
Copy link
Member Author

Multiple runs seems interesting, but keep in mind that the travis environment is virtualized - we don't have great control over how fast it is run to run. I think w/ this benchmarking solution we're just trying to get rough relative sizes of things.

@pq
Copy link
Member

pq commented May 19, 2017

The trouble with the current approach is that we're seeing wild variation when we run locally. (For example swings from ~75 to ~150ms for invariant_booleans -- more swings documented in dart-archive/linter#671). The best of 10 approach should tackle that. As for travis, I can't imagine it'll make things worse (though they will run longer...)

cc @alexeieleusis : dart-archive/linter#672 should be interesting to you 👍

@pq
Copy link
Member

pq commented May 22, 2017

We're live with basic benchmarking and one perf bug opened (#57575) 🤘

There are lots of possible next steps (better reporting, a more representative code base) but my gut says the first bit is done and we can close this in favor of follow-ups.

@devoncarew ?

@pq pq added the type-performance Issue relates to performance or code size label May 22, 2017
@devoncarew
Copy link
Member Author

says the first bit is done and we can close this in favor of follow-ups.

sounds reasonable to me. In terms of filing follow up issues, running it over a large representative codebase seems like a good one.

@pq pq closed this as completed May 22, 2017
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. type-performance Issue relates to performance or code size
Projects
None yet
Development

No branches or pull requests

3 participants