Options Validation source generator runs primary code generation phase on keypress

[ericstj]

Description

See more detail in #92914.

Reproduction Steps

Create a project that uses Options Validation source generator. Observe it's execution pattern - either in the debugger or through ETW.

Expected behavior

Changes unrelated to the options validation code and it's type closure should not trigger regeneration of the options source.

Actual behavior

Every change causes the entire pipeline to rerun.

Regression?

No

Known Workarounds

We haven't had reports of the performance here being a blocker, but that could be due to lack of use. The amount of work done on keypress will depend on whether or not the generator has work to do. If it has a lot of work to do, then it will be doing that work on every change.
 
Disable the options generator from design-time builds (this will result in errors where the generator is used, which are design time only errors).
Workaround:

  <Target Name="_disableOptionsGeneratorInDesignTime" BeforeTargets="ResolveOffByDefaultAnalyzers">
    <ItemGroup Condition="'$(DesignTimeBuild)' == 'true' OR '$(BuildingProject)' != 'true'">
      <OffByDefaultAnalyzer Include="Microsoft.Extensions.Options.SourceGeneration.dll"
                            IsEnabled="$(EnableOptionsGenerator)"/>
    </ItemGroup>
  </Target>

Configuration

No response

Other information

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-extensions-options
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

See more detail in #92914.

Reproduction Steps

Create a project that uses Options Validation source generator. Observe it's execution pattern - either in the debugger or through ETW.

Expected behavior

Changes unrelated to the options validation code and it's type closure should not trigger regeneration of the options source.

Actual behavior

Every change causes the entire pipeline to rerun.

Regression?

No

Known Workarounds

We haven't had reports of the performance here being a blocker, but that could be due to lack of use. The amount of work done on keypress will depend on whether or not the generator has work to do. If it has a lot of work to do, then it will be doing that work on every change.
 
Disable the options generator from design-time builds (this will result in errors where the generator is used, which are design time only errors).
Workaround:

  <Target Name="_disableOptionsGeneratorInDesignTime" BeforeTargets="ResolveOffByDefaultAnalyzers">
    <ItemGroup Condition="'$(DesignTimeBuild)' == 'true' OR '$(BuildingProject)' != 'true'">
      <OffByDefaultAnalyzer Include="Microsoft.Extensions.Options.SourceGeneration.dll"
                            IsEnabled="$(EnableOptionsGenerator)"/>
    </ItemGroup>
  </Target>

Configuration

No response

Other information

No response

Author:	ericstj
Assignees:	-
Labels:	untriaged, area-Extensions-Options, source-generator
Milestone:	-

[ericstj]

It looks like VS noticed the impact of this generator as well, causing 0.21% regression on a typing benchmark for razor pages. When fixing this we should be comparing performance to other well-behaved incremental generators (RegEx, Interop, Json) to ensure we raise enough of the data model to the incremental pipeline so that we don't end up calling the expensive part of our pipeline of every change.

[sharwell]

@ericstj PR #93427 will address (eliminate) the performance overhead for cases where the source generator is not being used. I'm watching this issue as tracking ongoing work to improve the performance specifically in cases where the generator is being used (i.e. produces at least one output).

[ericstj]

Yes, we need to improve that situation. FWIW we have the same class of bug in logging #93309 since 6.0 which also needs to be fixed.

As mentioned above the plan is to fix in 9.0 (with possibility of backport) - @sharwell do you know of a reason to prioritize a fix for this sooner than that?

[sharwell]

No, the urgent fix was already submitted.

[ericstj]

Thank you for confirming @sharwell. We will get to this in 9.0 I just wanted to make sure we were on the same page.