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

Use RevApi Code Extensions for Better Performance #33130

Conversation

alzimmermsft
Copy link
Member

@alzimmermsft alzimmermsft commented Jan 20, 2023

Description

At this time, we have a centralized RevApi configuration for all SDKs and due to having a single configuration it has a very large set of filters and transforms. Each filter and transform adds a non-trivial amount of overhead when running RevApi analysis as these configurations are turned into code filters and transform, from my understanding, on a one-to-one bases of configuration entry to operation performed in a list, meaning that with each run time for RevApi linting effectively grows linearly.

This PR changes portions of our RevApi configuration to begin using RevApi extension code where many filters and transforms are compacted into a single operation. The first change is using a single custom TreeFilterProvider to handle filtering out APIs from RevApi analysis. The change not only removes many filters that would have been created before it uses non-allocating String comparisons instead of regexes, greatly reducing CPU usage and memory allocations.

Reduces a RevApi only validation run in code-quality-reports from roughly 90-105 minutes to 30-35 minutes. Reduces Analyze stage of Spring pipelines from about 30 minutes to 15 minutes and Resource Manager pipelines from about 50 minutes to 25 minutes.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@alzimmermsft alzimmermsft added the EngSys This issue is impacting the engineering system. label Jan 20, 2023
@alzimmermsft alzimmermsft self-assigned this Jan 20, 2023
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@benbp
Copy link
Member

benbp commented Jan 25, 2023

/check-enforcer evaluate

@@ -51,7 +51,7 @@ public TransformationResult tryTransform(@Nullable E oldElement, @Nullable E new
return TransformationResult.keep();
}

if (!CORE_ARCHIVE.matcher(newArchive).matches()) {
if (!newArchive.startsWith("com.azure:azure-core:")) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: should strings like this be const values defined elsewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since they're only used in one location they don't explicitly need to be as the compiler will change them to constants in the class file.

@alzimmermsft
Copy link
Member Author

/azp run java - resourcemanager

@alzimmermsft
Copy link
Member Author

/azp run java - spring

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@alzimmermsft alzimmermsft merged commit 848670f into Azure:main Feb 13, 2023
@alzimmermsft alzimmermsft deleted the AzEng_UseCustomTreeFilterInsteadOfConfigurationFilters branch February 13, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EngSys This issue is impacting the engineering system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants