Comments (12)
Incremental generators include many steps in the graph. Each step is cached depending on its own inputs and outputs. Most changes to a project result in at least one step somewhere running. As best I can tell, the proposed GroupBy
operation would not inherently make anything cacheable that is not already possible to cache today using Collect
and SelectMany
. It's also possible that Collect().Select()
has good caching characteristics. The link I gave above includes a step with that form.
from roslyn.
Am I correct in understanding that calling .Collect() makes an IncrementalGenerator non-cache friendly? I think I may be running into this issue when I collect to do some deduping.
from roslyn.
You can call Collect()
and then split the items back apart using SelectMany()
, and the only part that runs each time is the operation within the Collect()
. Here's an example:
https://github.com/dotnet/roslyn-analyzers/blob/7455f63411369a962f769361d1a979a547756ada/src/Microsoft.CodeAnalysis.ResxSourceGenerator/Microsoft.CodeAnalysis.ResxSourceGenerator/AbstractResxGenerator.cs#L131-L175
GroupBy
could be efficiently implemented by using Collect()
followed by SelectMany
, where each element of the SelectMany
is a group.
from roslyn.
So provider.Collect().Select()
that outputs an immutable array provider would not be cacheable, but a provider.Collect().SelectMany()
would produce something cacheable?
from roslyn.
.Collect
is fine to call, you just need to provide a comparer to properly compare the arrays it has inside of it. Note: i consider it bad that this is needed, but c'est la vie for now. But as long as you give an appropriate comparer, things will be incrementally safe and sound.
from roslyn.
@chsienki I think we really need to invest in:
- a comparer for comparing ImmutableArrays.
- having anything we produce that returns a
IncrementalValueProvider<ImmutableArray<TSource>>
use teh comparer.
There's really no reason at all to have no comparer here, and it means a collect call will break incrementality unless a user knows to go fix this up.
from roslyn.
@CyrusNajmabadi I'm not sure that aspect is a problem today. I've seen many source generators opt for EquatableArray<T>
instead of ImmutableArray<T>
in their data models. It's not clear how the current behavior of Collect()
would have any negative impact on this.
Providing a default comparer for ImmutableArray<T>
would not address cases where EquatableArray<T>
is needed inside of record types, and if you already use EquatableArray<T>
in the model it's not clear why ImmutableArray<T>
would be needed separately.
from roslyn.
@sharwell Collect
doesn't use that though, ti's just:
public static IncrementalValueProvider<ImmutableArray<TSource>> Collect<TSource>(this IncrementalValuesProvider<TSource> source) => new IncrementalValueProvider<ImmutableArray<TSource>>(new BatchNode<TSource>(source.Node), source.CatchAnalyzerExceptions);
So you get an IVP
(not an IVsP
) point at an ImmutableArray
without a proper comparer for the immutable array itself. .Collect doesn't let the user provide the final array the values are collected into. It chooses that it is IA<>
, and you need to know to provide an appropriate comparer if you want proper incrementality at that pipeline step.
from roslyn.
You can call
Collect()
and then split the items back apart usingSelectMany()
, and the only part that runs each time is the operation within theCollect()
. Here's an example:
So if I'm understanding the semantics of SelectMany
correctly, it is forking one IVP into an IVsP of independently cached values? I think that could work for my use case.
from roslyn.
.Collect doesn't let the user provide the final array the values are collected into
That should be fine. If none of the inputs to the Collect
operation changed, Collect
should be reusing the old cached ImmutableArray<>
output instead of creating a new one. The only time Collect
produces a new ImmutableArray<>
to compare with the old cached value is when one or more elements in that array no longer compare element-wise equal with the old values.
and you need to know to provide an appropriate comparer if you want proper incrementality at that pipeline step
There is no public API to alter the comparer for Collect
, but that's fine because the default comparer is correct for the semantics of this operation.
from roslyn.
There is. You call . WithComparer afterwards
from roslyn.
The only time Collect produces a new ImmutableArray<> to compare with the old cached value is when one or more elements in that array no longer compare element-wise equal with the old values.
Right. That's the exact problem. You get a different input, but you end up creating conceptually the same output (which is very very possible). As far as the driver is concerned, because it's a new IA, it is different, and it will not stop the incremental pipeline.
from roslyn.
Related Issues (20)
- IDE0130 warning "Namespace 'Foo.Bar' does not match folder structure, expected 'Foo_xyz_wpftmp.Bar'" when EnforceCodeStyleInBuild is enabled
- Bad performance in CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer with System.Text.Json source generator HOT 12
- [Automated] PRs inserted in VS build main-35214.134
- Use interpolated string handler for string concat HOT 3
- Using SuppressMessage on a const declaration causes Diagnostic analyzer runner interal error
- `CSharpWrappingCodeRefactoringProvider` encountered an error and has been disabled
- `CSharpMergeNestedIfStatementsCodeRefactoringProvider` encountered an error and has been disabled
- `CSharpMergeConsecutiveIfStatementsCodeRefactoringProvider` encountered an error and has been disabled
- Usage of Activator.CreateInstance won't show in references on the constructor, so there is misleading info!
- Automatic exception xml tag when using static class such as ArgumentNullException HOT 2
- Allow the Visual Studio "Sync Namespaces" tool to work at the folder level, not just Project and Solution.
- Refactoring suggestion -> Move to new file needs a confirmation dialog
- Reenable BC30645ERR_InvalidOptionalParameterUsage1b
- [Automated] PRs inserted in VS build feature.debugger.main-35215.202
- Inconsistent report of CS8156 when using generically constrained interfaces and [UnscopedRef]
- Sourcelink does not debug merged partial classes
- Advanced analysis options are unreliable, confusing and poorly documented
- EditorConfig "modern" vs legacy priority revert related documentation updates
- CS8344 error even with C# 13 (ref in iterators/async) HOT 1
- Editor and debugger don't agree on breakpoint range
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from roslyn.