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.

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

Speedup conversion time for code without "MyClass" and "WithEvents / Handles" #1090

Open
LucTremblay opened this issue Apr 10, 2024 · 3 comments

Comments

@LucTremblay
Copy link

Hi, I am looking for a way to speed up my conversion time.

I looked at ours overall busniness code and we only have a few utilisation of the key words "MyClass" and "WithEvents / Handles" that we can take care manually.

I would like to know your advice, if we could safely remove the following from DeclarationNodeVisitor.cs, as it's seems to cut in half (if not more) the time it takes to convert, especially the WithEvents / Handles part.

MyClass

    private static HashSet<string> GetMyClassAccessedNames(VBSyntax.ClassBlockSyntax classBlock)
    {
        //var memberAccesses = classBlock.DescendantNodes().OfType<VBSyntax.MemberAccessExpressionSyntax>();
        //var accessedTextNames = new HashSet<string>(memberAccesses
            //.Where(mae => mae.Expression is VBSyntax.MyClassExpressionSyntax)
            //.Select(mae => mae.Name.Identifier.Text), StringComparer.OrdinalIgnoreCase);
        //return accessedTextNames;
        return new HashSet<string>();
    }

WithEvents / Handles

    private async Task<HandledEventsAnalysis> GetMethodWithHandlesAsync(VBSyntax.TypeBlockSyntax parentType, IMethodSymbol designerGeneratedInitializeComponentOrNull)
    {
        //if (parentType == null || _semanticModel.GetDeclaredSymbol((SyntaxNode)parentType) is not INamedTypeSymbol containingType) {
        return new HandledEventsAnalysis(CommonConversions, null, Array.Empty<(HandledEventsAnalysis.EventContainer EventContainer, (IPropertySymbol Property, bool IsNeverWrittenOrOverridden) PropertyDetails, (EventDescriptor Event, IMethodSymbol HandlingMethod, int ParametersToDiscard)[] HandledMethods)>());
        //}
        //return await HandledEventsAnalyzer.AnalyzeAsync(CommonConversions, containingType, designerGeneratedInitializeComponentOrNull, _typeToInheritors);
    }

Thank you

@GrahamTheCoder
Copy link
Member

Sure, I don't see any immediate issue there.

Have you run the whole thing twice already and found that difference? Or do you just mean the first phase seemed faster when partially tested with this on a few files?

Just interested in the likelihood this would help others since it may be possible to speed up these checks. Usually the second phase is so much slower that I haven't bothered much to optimise the first phase.

@LucTremblay
Copy link
Author

Yes, I did ran it with and without it, on many projects, and the whole thing ran at least 2 times faster.

Some projects were taking a really long time, and I added a Stopwatch to give the conversion time and simplification time of every file.

I noticed that only a few files were taking a long time, and most of them where taking exaclly the same time to convert.

The common thing between them was a base class used by a lot of other project, in the solution.

It seams to take a long time in this function:

  private async Task<bool> IsNeverWrittenOrOverriddenAsync(ISymbol symbol)
  {
      var projectSolution = _commonConversions.Document.Project.Solution;
      if (!await projectSolution.IsNeverWrittenAsync(symbol, _initializeComponentLocationOrNull)) return false;
      return !_typeToInheritors.Contains(symbol.ContainingType);
  }

The fonction seams to be called on every property of the current converted type and its base class, looking for its usage in the whole solution and we have a solution of 300 projects with over 400 000 lines of code, so this migth be the reason why it takes a long times for us.

Combined to the fact that, this long operation is repeated for every types ineriting the widely use class.

@GrahamTheCoder
Copy link
Member

GrahamTheCoder commented Apr 13, 2024

Thanks that's some great context!
At a glance, I'm wondering if those two checks can be swapped in order since the _typeToInheritors one should be much faster.
Though I was already aware projectSolution.IsNeverWrittenAsync was pretty slow, so I'm surprised I haven't already done so if possible.

EDIT: It would only be faster in the case when _typeToInheritors.Contains is true, which is relatively rare, though anyone's welcome to PR the change if it makes a difference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants