Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Optimize certain operations #1206

Merged
merged 6 commits into from
Apr 20, 2023
Merged

Optimize certain operations #1206

merged 6 commits into from
Apr 20, 2023

Conversation

jensjoha
Copy link
Contributor

I noticed that formating the file "pkg/front_end/lib/src/fasta/fasta_codes_cfe_generated.dart" from the dart sdk was quite slow, took a look into it and made some changes.
It's still quite slow, but at least it's better.

I've:

  • Updated _compareScore to "cache" the costs and inlined the compareTo call (the sdk version takes a num and don't know it's already been compared to 0).
  • Replaced the usage of Set in a few places with markings on the actual classes.

My measured runtimes:

AS-IS
Formatted 1 file (0 changed) in 3.53 seconds.
Formatted 1 file (0 changed) in 3.65 seconds.
Formatted 1 file (0 changed) in 3.64 seconds.
Formatted 1 file (0 changed) in 3.70 seconds.
Formatted 1 file (0 changed) in 3.85 seconds.
Formatted 1 file (0 changed) in 3.70 seconds.
Formatted 1 file (0 changed) in 3.76 seconds.
Formatted 1 file (0 changed) in 3.59 seconds.
Formatted 1 file (0 changed) in 3.77 seconds.
Formatted 1 file (0 changed) in 3.80 seconds.

_compareScore change
Formatted 1 file (0 changed) in 3.21 seconds.
Formatted 1 file (0 changed) in 3.35 seconds.
Formatted 1 file (0 changed) in 3.28 seconds.
Formatted 1 file (0 changed) in 3.10 seconds.
Formatted 1 file (0 changed) in 3.06 seconds.
Formatted 1 file (0 changed) in 3.35 seconds.
Formatted 1 file (0 changed) in 3.39 seconds.
Formatted 1 file (0 changed) in 3.08 seconds.
Formatted 1 file (0 changed) in 3.15 seconds.
Formatted 1 file (0 changed) in 3.31 seconds.

Difference at 95.0% confidence
        -0.471 +/- 0.105388
        -12.7332% +/- 2.84909%
        (Student's t, pooled s = 0.112163)

Mark Spans instead of using set in _calculateCost
Formatted 1 file (0 changed) in 2.96 seconds.
Formatted 1 file (0 changed) in 2.92 seconds.
Formatted 1 file (0 changed) in 2.98 seconds.
Formatted 1 file (0 changed) in 2.99 seconds.
Formatted 1 file (0 changed) in 3.10 seconds.
Formatted 1 file (0 changed) in 3.20 seconds.
Formatted 1 file (0 changed) in 2.93 seconds.
Formatted 1 file (0 changed) in 2.84 seconds.
Formatted 1 file (0 changed) in 3.09 seconds.
Formatted 1 file (0 changed) in 3.05 seconds.

Difference at 95.0% confidence
        -0.222 +/- 0.107951
        -6.87732% +/- 3.34422%
        (Student's t, pooled s = 0.114891)

Mark NestingLevels instead of using set in _calculateSplits
Formatted 1 file (0 changed) in 2.77 seconds.
Formatted 1 file (0 changed) in 2.97 seconds.
Formatted 1 file (0 changed) in 2.86 seconds.
Formatted 1 file (0 changed) in 2.90 seconds.
Formatted 1 file (0 changed) in 3.01 seconds.
Formatted 1 file (0 changed) in 2.83 seconds.
Formatted 1 file (0 changed) in 2.88 seconds.
Formatted 1 file (0 changed) in 2.88 seconds.
Formatted 1 file (0 changed) in 2.88 seconds.
Formatted 1 file (0 changed) in 2.90 seconds.

Difference at 95.0% confidence
        -0.118 +/- 0.0826868
        -3.92548% +/- 2.75073%
        (Student's t, pooled s = 0.0880025)

Which is a combined changed of

Difference at 95.0% confidence
        -0.811 +/- 0.079311
        -21.9248% +/- 2.14412%
        (Student's t, pooled s = 0.0844097)

i.e ~800 ms or almost 22% faster on this - apparently particularly bad - file.

munificent
munificent requested changes Apr 18, 2023
View reviewed changes
Copy link
Member

@munificent munificent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. I ran it on a larger corpus and it seems to improve performance slightly across the board. Thanks for hacking on it.

lib/src/line_splitting/solve_state.dart Outdated Show resolved Hide resolved
lib/src/line_splitting/solve_state_queue.dart Outdated Show resolved Hide resolved
lib/src/marking_scheme.dart Outdated Show resolved Hide resolved
lib/src/marking_scheme.dart Outdated Show resolved Hide resolved
lib/src/marking_scheme.dart Outdated Show resolved Hide resolved
munificent
munificent approved these changes Apr 20, 2023
View reviewed changes
Copy link
Member

@munificent munificent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@munificent munificent merged commit e7ce3a2 into dart-lang:main Apr 20, 2023
munificent added a commit that referenced this pull request May 12, 2023
@munificent
Merge branch 'main' into flutter-style-experiment
804288e 
This does not include the optimizations to SolveState from:

#1206

Those were too hard to merge over since the cost calculation code was
moved to a separate file. Also, I'm not sure if those optimizations are
still relevant given all of the other changes.

# Conflicts:
#	lib/src/line_splitting/solve_state.dart
#	lib/src/line_writer.dart
#	lib/src/rule/argument.dart
#	lib/src/source_visitor.dart
#	pubspec.lock
#	test/comments/expressions.stmt
#	test/splitting/assignments.stmt
#	test/splitting/type_arguments.stmt
#	test/whitespace/metadata.unit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@munificent munificent munificent approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jensjoha @munificent