Speed up use_late_for_private_fields_and_variables

[srawlins]

Description

The gist is that UnifyingAstVisitor was a little expensive, as we ultimately only do any work if we're looking at (1) the left side of an AssignmentExpression, or (2) an Identifier or a PropertyAccess. So it wasn't too hard to just visit those nodes with a RecursiveAstVisitor.

Also in visitFieldDeclaration, I moved the parent != null check to be outside the for loop

I also tidied up a few comments to be formatted like sentences.

Fixes dart-lang/sdk#58815

Does not address the P1 memory leak or the false positive bug.

[coveralls]

Coverage decreased (-0.09%) to 95.532% when pulling c31466b on speed-up-late into 024e8a1 on main.

[pq]

Whoa. Checkout these benchmark comparisons.

Before:

After:

That's a big improvement!

@a14n: I'm curious about the rationale for using a unifying visitor... Maybe this made more sense with a previous iteration of the source?

[a14n]

@a14n: I'm curious about the rationale for using a unifying visitor... Maybe this made more sense with a previous iteration of the source?

TBH I don't remember and TBH2 I didn't think there was this perf diff between UnifyingAstVisitor and RecursiveAstVisitor

[srawlins]

On my machine I also noticed a 50% improvement 🎉

[pq]

💎

[bwilkerson]

This is still concerning. The visitors added to the registry (line 73) should implement SimpleAstVisitor. With this particular code pattern we're visiting the AST twice, which is less efficient than just registering for the visit methods implemented in the visitor. (I didn't look in depth to understand if there's a reason that wouldn't work here.)