Split Ark.Tools.Core into trimmable Core and non-trimmable Core.Reflection packages#586
Merged
AndreaCuneo merged 11 commits intomasterfrom Jan 13, 2026
Merged
Conversation
Copilot
AI
changed the title
[WIP] Update plan for trimmable support documentation
Split Ark.Tools.Core into trimmable Core and non-trimmable Reflection packages
Jan 11, 2026
AndreaCuneo
requested changes
Jan 12, 2026
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
AndreaCuneo
requested changes
Jan 13, 2026
Contributor
AndreaCuneo
left a comment
AndreaCuneo
left a comment
There was a problem hiding this comment.
rename the library as Ark.Tools.Core.Reflection. add the library to root solution.
Contributor
Author
✅ Renamed to
Commit: 359feaa Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Split Ark.Tools.Core into trimmable Core and non-trimmable Reflection packages
Split Ark.Tools.Core into trimmable Core and non-trimmable Core.Reflection packages
Jan 13, 2026
AndreaCuneo
requested changes
Jan 13, 2026
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
AndreaCuneo
force-pushed
the
copilot/update-docs-trimmable-support
branch
from
41e981e to
219d712
Compare
Contributor
|
@copilot rebuild the solution and update the packages.lock.json Review the Shred to datatable utility: given it has T generic, it should be possible to annotate it with DynamicAccessMembers and move back to Ark.Tools.Core. Check and report. |
Contributor
Author
✅ Part 1 - Rebuild Complete: Updated packages.lock.json (Commit: 2a395b0). Build: 0 warnings, 0 errors. ❌ Part 2 - ShredObjectToDataTable Analysis: Cannot be moved to Core - not trimmable even with annotations. Key Issue: Uses DataKey comparison:
Additionally, Recommendation: Keep in Core.Reflection. This is genuinely reflection-heavy code with polymorphic type handling that's incompatible with trimming. |
Contributor
|
@copilot ShredDataTable has more fearures than currently needed, in particular the part related to runtime types and thus support inherited objects. Fork the current implementation. Create a new Shred implementaion forked of the existing one but without the "ExtendTable" and thus Runtime type, using only the GetProps and GetFields on the T. Move this "strip" down version back to Ark.Tools and add a new .ToDatatable() extension. Annotate this in v6 migration as Breaking change as the "original" ToDatatable() now don't support polymorphic enumerables. Guide users to use the Todatatable from .Reflection Rename the ToDatatable in .Reflection to ToDatatablePolymorphic() |
Contributor
Author
✅ Completed all requested changes:
Result: ResourceWatcher.Sql is now trim-compatible! 0 warnings, 0 errors, all 117 tests pass. Commit: b1d1f3b Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
Author
The commit b1d1f3b is on the branch. Verified: Full hash: The commit contains all changes:
Try refreshing the PR page or run |
AndreaCuneo
requested changes
Jan 13, 2026
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
AndreaCuneo
requested changes
Jan 13, 2026
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
AndreaCuneo
approved these changes
Jan 13, 2026
…tation After enabling trimming and analyzing all warnings, determined Ark.Tools.Core is fundamentally not trimmable due to extensive reflection usage. Generated 88 trim warnings across 7 files. Key reflection features: - ShredObjectToDataTable<T> (32 warnings) - LINQ Queryable extensions (24 warnings) - ReflectionHelper utilities (16 warnings) - DynamicTypeAssembly (8 warnings) - Enum/DataKey reflection (8 warnings) Added comprehensive README_TRIMMING.md documenting why each feature is not trimmable, which parts are safe in trimmed apps, and alternatives considered. Impact: Despite Core not being trimmable, 35/42 libraries (83%) are now trimmable. Dependent libraries can still be marked trimmable if they use non-reflection parts. Closes the common libraries phase - all 42 libraries analyzed and documented! Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
…ation) Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
…not trimmable) Created new Ark.Tools.Reflection library for reflection-heavy code. Files moved to Reflection: - ShredObjectToDataTable.cs - ReflectionHelper.cs (most methods) - DynamicTypeAssembly.cs - EnumerableExtensions.cs (Queryable methods) - DataKey/DataKeyComparer.cs, DataKeyPrinter.cs - EnumExtensions.AsString (GetField reflection) Files kept in Core (trim-safe): - DataKeyAttribute - EnumExtensions.ParseEnum Updated all dependent projects with Reflection references where needed. ISSUE IDENTIFIED: SystemTextJson references Reflection which defeats trimming purpose. Next: Move GetCompatibleGenericInterface/BaseClass back to Core with proper annotations. Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
…p SystemTextJson trimmable Moved 2 methods from Reflection back to Core with proper annotations: - GetCompatibleGenericInterface (with [DynamicallyAccessedMembers.Interfaces]) - GetCompatibleGenericBaseClass (already trim-safe, no annotation needed) This allows SystemTextJson to remove its Reflection dependency and remain trimmable. Added suppressions to GenericDictionaryWithConvertibleKey for IL2067 warnings since base class JsonConverterFactory doesn't have DynamicallyAccessedMembers annotations. Result: Ark.Tools.SystemTextJson is now TRIMMABLE and doesn't pull in reflection-heavy code! Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
…oper trim annotations Addressed PR review feedback by moving trim-safe methods back to Ark.Tools.Core: 1. EnumExtensions.AsString<T>: - Added [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] to T - This preserves enum fields for GetField() and GetCustomAttributes() calls - Now trim-safe and available in Core 2. ReflectionHelper.GetAttribute methods: - Moved both overloads (MemberInfo and Type) to Core - Type.GetCustomAttributes() on Type itself doesn't require annotation - MemberInfo.GetCustomAttributes() works without annotation - Both are trim-safe 3. Removed duplicates from Ark.Tools.Reflection: - Deleted EnumExtensions.cs (AsString now in Core) - Removed GetAttribute, GetCompatibleGenericInterface, GetCompatibleGenericBaseClass from Reflection.ReflectionHelper - Kept reflection-heavy methods (GetEnumerableItemType, GetFieldOrPropertyValue, GetCSTypeName) in Reflection Result: More functionality now available in trimmable Core package without pulling in Reflection! Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
…d add to solution Addressed PR feedback: 1. Renamed library from Ark.Tools.Reflection to Ark.Tools.Core.Reflection - Updated directory name - Updated csproj file name - Updated all namespaces from Ark.Tools.Reflection to Ark.Tools.Core.Reflection - Updated namespace for DataKey subfolder to Ark.Tools.Core.Reflection.DataKey 2. Added Ark.Tools.Core.Reflection to root solution (Ark.Tools.slnx) - Placed after Ark.Tools.Core in alphabetical order 3. Updated all project references: - Ark.Tools.AspNetCore - Ark.Tools.ResourceWatcher.Sql - Ark.Reference.Common - Directory.Packages.props in samples 4. Updated all using statements: - ArkProblemDetailsOptionsSetup.cs - SqlStateProvider.cs - ArkStartupWebApiCommon.cs (fully qualified name) 5. Removed README_TRIMMING.md from Ark.Tools.Core (no longer applicable after split) Result: Clean build with zero errors, all 115 tests pass! Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
…embers annotations Addressed PR feedback to make DataKey classes trim-safe: DataKey classes only use GetProperties() and GetCustomAttributes() which can be made trim-safe with [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] annotation on the generic type parameter T. Changes: - Moved DataKeyComparer<T> from Core.Reflection to Core - Moved DataKeyPrinter<T> from Core.Reflection to Core - Moved DataKeyHashSet<T> from Core.Reflection to Core - Added [DynamicallyAccessedMembers.PublicProperties] to all T generic parameters - Updated namespaces from Ark.Tools.Core.Reflection.DataKey to Ark.Tools.Core.DataKey - Removed DataKey folder from Ark.Tools.Core.Reflection Result: DataKey utilities now in trimmable Core package with zero warnings! Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
Rebuilt the full solution to ensure all dependencies are properly resolved. Updated packages.lock.json for Ark.Tools.Core.Reflection. Build result: 0 warnings, 0 errors Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
BREAKING CHANGE: ToDataTable() in Ark.Tools.Core no longer supports polymorphic types Created a simplified, trim-safe version of ShredObjectToDataTable in Ark.Tools.Core: - Only supports exact type T (not derived types) - Annotated with [DynamicallyAccessedMembers] for trim safety - Removed ExtendTable and runtime type handling - Uses suppression for IL2072 in DeriveColumnType (returns known safe types) Renamed polymorphic version in Ark.Tools.Core.Reflection: - ToDataTable() -> ToDataTablePolymorphic() - Added [Obsolete] to old ToDataTable() methods with migration guidance - Supports derived types but not trim-compatible Updated Ark.Tools.ResourceWatcher.Sql: - Changed to use trim-safe ToDataTable() from Core - Removed Core.Reflection dependency (now trim-compatible!) - Uses anonymous types which are exact types (no polymorphism needed) Migration Guide: - For trim-safe apps: Use ToDataTable() from Ark.Tools.Core (exact types only) - For polymorphic support: Use ToDataTablePolymorphic() from Ark.Tools.Core.Reflection - ResourceWatcher.Sql is now trim-compatible Result: 0 warnings, 0 errors, all 117 tests pass! Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
…ataTableArk Addressed PR review feedback: 1. Removed 3 Obsolete methods from Core.Reflection (ToDataTableArk, ToDataTable x2) 2. Made ShredObjectToDataTable static class with static fields (_fi, _pi) 3. Removed parameterless constructor (fields now initialized inline) 4. Changed InitializeTable to return FrozenDictionary<string, int> ordinalMap 5. Ordinal map now computed per-table to use actual column ordinals 6. Added ToDataTableArk() method to Core (trim-safe version) 7. Updated SqlStateProvider to use ToDataTableArk() Performance improvements: - Fields are static per generic type T (shared across all instances) - No constructor overhead - Ordinal map uses FrozenDictionary for better performance - Ordinal map computed only once per Shred() call Result: 0 warnings, 0 errors, all 116 tests pass! Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
…ensions Addressed PR review feedback: 1. Added TODO document for optimizing EnumerableExtensions OrderBy string parsing - Created docs/todo/optimize-enumerable-extensions-orderby.md - Documents using Span-based alternatives (MemoryExtensions) instead of Split()/Trim() - Reduces allocations and improves performance 2. Optimized DataTableExtensions for null table parameter - Fast path: When table is null, initialize with fields then properties in order - No ordinal map needed for new tables (sequential column ordering) - Slow path: When table exists, compute ordinal map once before loop - Added InitializeNewTable() for sequential column addition - Added ShredObjectSequential() for fast sequential value extraction - Renamed InitializeTable() to GetOrdinalMap() for clarity - Handles field/property name collisions correctly Performance improvements: - New tables: No FrozenDictionary allocation, direct sequential access - Existing tables: Ordinal map computed once per Shred() call, not per row - Preserves exact ordering: fields first, then properties Result: 0 warnings, 0 errors, all 117 tests pass! Co-authored-by: AndreaCuneo <5227688+AndreaCuneo@users.noreply.github.com>
AndreaCuneo
force-pushed
the
copilot/update-docs-trimmable-support
branch
from
a57d67f to
dd26946
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trimmable Support - Performance Optimizations Complete ✅
Summary
Optimized DataTableExtensions and added TODO documentation for future EnumerableExtensions optimization. All 117 tests pass!
Changes Made (Addressing PR Comments)
✅ Comment #2686076860: TODO for EnumerableExtensions Optimization
Created comprehensive TODO document:
docs/todo/optimize-enumerable-extensions-orderby.mdOptimization Goal: Replace string operations with Span-based alternatives
Current Issues:
Split()allocates string arraysTrim()allocates trimmed stringsProposed Solution:
ReadOnlySpan<char>for parsingMemoryExtensions.Split()andTrim()Substring()Benefits:
✅ Comment #2686129481: Optimized DataTableExtensions
Removed ordinal map when table is null:
Before:
After - Two Paths:
Fast Path (table == null):
Slow Path (table != null):
Key Optimizations:
table.Columns.Contains()in both pathsImplementation Details:
InitializeNewTable():
ShredObjectSequential():
values[index++]GetOrdinalMap() (renamed from InitializeTable):
Performance Improvements
New Tables (Fast Path):
Existing Tables (Slow Path):
Benchmark Estimate (for 1000 rows):
Files Modified
New Files:
docs/todo/optimize-enumerable-extensions-orderby.md- TODO documentModified Files:
src/common/Ark.Tools.Core/DataTableExtensions.cs:Shred()into two paths (fast/slow)InitializeNewTable()for sequential column creationShredObjectSequential()for fast value extractionInitializeTable()→GetOrdinalMap()Verification
✅ Build: 0 warnings, 0 errors
✅ Tests: All 117 tests pass (1 more than previous commit!)
✅ Trim warnings: 0 (Core remains trimmable)
✅ Behavior: Exact same output as before (ordering preserved)
Breaking Changes
None - all optimizations are internal implementation details.
Next Steps
Future optimization (from TODO document):
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.