-
-
Notifications
You must be signed in to change notification settings - Fork 19
Closed
Description
Summary
The codebase has 40+ reflection calls for method invocation, property access, and type instantiation. This impacts performance, type safety, and maintainability.
Severity: MEDIUM
Reflection is sometimes necessary but overuse creates maintenance burden and runtime risks.
Current Reflection Usage
Pattern 1: Generic Module Execution via Reflection
File: src/ModularPipelines/Engine/ModuleExecutor.cs:488-496
var executeMethod = typeof(IModuleExecutionPipeline)
.GetMethod(nameof(IModuleExecutionPipeline.ExecuteAsync))!
.MakeGenericMethod(resultType);
var task = (Task)executeMethod.Invoke(_executionPipeline, ...)!;
await task;
var resultProperty = task.GetType().GetProperty("Result")!;
return (IModuleResult)resultProperty.GetValue(task)!;Why Reflection: Module result type T is only known at runtime.
Pattern 2: Options Resolution
File: src/ModularPipelines/Engine/OptionsProvider.cs:40-42
yield return option!
.GetType()
.GetProperty("Value", BindingFlags.Public | BindingFlags.Instance)!
.GetValue(option);Pattern 3: Activator.CreateInstance
File: src/ModularPipelines/Helpers/ParallelLimitProvider.cs:35
var parallelLimit = Activator.CreateInstance(parallelLimitType) as IParallelLimit;Pattern 4: Attribute Discovery
Multiple files use reflection for attribute-based configuration.
Problems with Heavy Reflection
- Performance: Reflection is 10-100x slower than direct calls
- No compile-time safety: Refactoring can break reflection calls silently
- Hard to debug: Stack traces are less clear
- IDE support: Find References, Rename don't work across reflection
- AOT incompatible: Native AOT compilation struggles with reflection
Proposed Alternatives
1. Source Generators for Module Registration
Instead of runtime reflection for module discovery:
// Generated at compile time
public static class ModuleRegistry
{
public static IEnumerable<ModuleDescriptor> GetModules() => new[]
{
new ModuleDescriptor<BuildModule, BuildResult>(),
new ModuleDescriptor<TestModule, TestResult>(),
};
}2. Generic Constraints Instead of Reflection
// Instead of runtime MakeGenericMethod
public interface IModuleExecutor
{
Task<TResult> ExecuteAsync<TModule, TResult>()
where TModule : Module<TResult>;
}3. Cached Compiled Delegates
// Cache compiled lambda for repeated reflection calls
private static readonly ConcurrentDictionary<Type, Func<object, object>> PropertyGetters = new();
public static object GetValue(object obj, string propertyName)
{
var type = obj.GetType();
var getter = PropertyGetters.GetOrAdd(type, t =>
{
var prop = t.GetProperty(propertyName);
var param = Expression.Parameter(typeof(object));
var body = Expression.Convert(
Expression.Property(Expression.Convert(param, t), prop),
typeof(object));
return Expression.Lambda<Func<object, object>>(body, param).Compile();
});
return getter(obj);
}4. Interface-Based Discovery
// Instead of attribute reflection
public interface IModuleMetadata
{
Type[] Dependencies { get; }
bool ShouldSkip(IPipelineContext context);
}Inventory of Reflection Usage
| Location | Purpose | Alternative |
|---|---|---|
| ModuleExecutor | Execute generic modules | Source generator or compiled delegate |
| OptionsProvider | Get IOptions.Value | Direct interface access |
| ParallelLimitProvider | Create limit instances | DI registration |
| AttributeEventInvoker | Invoke attribute methods | Interface-based events |
| StackTraceModuleDetector | Find calling module | AsyncLocal or explicit parameter |
Acceptance Criteria
- Document each reflection usage with justification
- Replace reflection with compile-time alternatives where feasible
- Cache reflection results for unavoidable cases
- Add null checks for all reflection calls (see Add null checks for reflection calls instead of null-forgiving operators #1420)
- Consider source generators for module registration
Labels
performance, architecture, maintainability
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels