Skip to content

Breakdown comment reference handling further and tweak performance #1863

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

Merged
merged 4 commits into from
Dec 7, 2018

Conversation

jcollins-g
Copy link
Contributor

Minor improvement to comment reference handling; this now makes comment reference handling overall negligible for performance (~ 5 seconds spent on a flutter run now) and hopefully reduces further the amount of cut-and-pastyness going on in the reference handling. It should make it easier when we go in to rewrite that to understand what the current code does.

In addition, disable by default the high quality stack traces inside the Chain.capture() call. This has a more significant performance impact.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Dec 7, 2018
@jcollins-g jcollins-g requested review from devoncarew and pq December 7, 2018 01:17
Copy link
Member

@pq pq left a comment

Choose a reason for hiding this comment

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

Nice!

Added a silly aside but not really pertinent and certainly not suggesting a change.

@@ -6568,8 +6603,6 @@ class PackageBuilder {
PerformanceLog log = new PerformanceLog(null);
AnalysisDriverScheduler scheduler = new AnalysisDriverScheduler(log);
AnalysisOptionsImpl options = new AnalysisOptionsImpl();
options.enableSuperMixins = true;
options.previewDart2 = true;

Copy link
Member

Choose a reason for hiding this comment

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

🎉

// because of accessors and operators.
if (_namePart == null) {
_namePart = fullyQualifiedName.split('.').last;
}
Copy link
Member

Choose a reason for hiding this comment

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

Silly, and kind of an aside but I'm curious nonetheless about shorthand (since I do this kind of lazy init all the time).

How crazy is something like this?

return _namePart ??= fullyQualifiedName.split('.').last;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll use that but I now do it only sparingly for really trivial things (e.g. ??= super.foo). The reason is threefold; one, ??= is kind of obscure and even after programming in Dart for a couple years I still pause for a second when I see it.

Two, if I need to add debugging code for any reason, or want to add something more complex to the statement, I have to add all the boilerplate then rather than concentrate on what I was doing, and then remember to remove it.

And finally, since there are plenty of cases where ??= isn't appropriate stylistically anyway (if we're doing more than one or two lines of work), I prefer to have this idiom look the same wherever I see it. This way I can concentrate on important things and let this fade into the background by making use of my human brain's ability to process repetitive sequences subconsciously. Dartdoc isn't quite consistent but I'd like it to be eventually.

Copy link
Member

Choose a reason for hiding this comment

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

Cool. Thanks for the reflections!

@jcollins-g jcollins-g merged commit b7b0eb0 into master Dec 7, 2018
@kevmoo kevmoo deleted the perf-check branch January 18, 2019 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants