Skip to content

Fix review comments for CSV string.Split benchmarks#5202

Closed
Copilot wants to merge 3 commits intomainfrom
copilot/fix-comments-in-review-thread
Closed

Fix review comments for CSV string.Split benchmarks#5202
Copilot wants to merge 3 commits intomainfrom
copilot/fix-comments-in-review-thread

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 11, 2026

Addresses all review comments from the CSV string.Split benchmark PR. Merges the original changes and applies the requested fixes.

Changes

  • Shorter CSV filenames: Renamed long-named files to avoid Windows path-length build failures. Original source URLs preserved in comments and THIRD-PARTY-NOTICES.TXT.

    • Monthly_Motor_Fuel_Sales_Reported_by_States__Selected_Data_from_FHWA_Monthly_Motor_Fuel_Report.csvmotor_fuel_sales.csv
    • 500_Cities__City-level_Data__GIS_Friendly_Format___2019_release.csv500_cities.csv
  • Cached file I/O: CsvCorpus() previously called ReadInputFile() inline on every enumeration (BDN enumerates ArgumentsSource multiple times during discovery and per job). Replaced with static Lazy<string[]> fields loaded once on first access:

private static readonly Lazy<string[]> s_csvShortText = new Lazy<string[]>(() => ReadInputFile("TMEDTREND_PUBLIC_240528.csv"));
private static readonly Lazy<string[]> s_csvShortMixed = new Lazy<string[]>(() => ReadInputFile("motor_fuel_sales.csv"));
// ...

public static IEnumerable<object[]> CsvCorpus()
{
    yield return new object[] { "Short Text", s_csvShortText.Value };
    yield return new object[] { "Short Mixed", s_csvShortMixed.Value };
    // ...
}
  • Style fix: new object []{ ... }new object[] { ... } to match surrounding entries.

  • Third-party notices: Added THIRD-PARTY-NOTICES.TXT documenting all six CSV sources (CMS, DOT, Census Bureau, USDA, EPA, CDC — all U.S. government public domain data).

Copilot AI and others added 2 commits April 11, 2026 22:49
Agent-Logs-Url: https://github.com/dotnet/performance/sessions/5a3db2a2-dd61-41bc-9847-4083906d08a5

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
… style, add THIRD-PARTY-NOTICES.TXT

Agent-Logs-Url: https://github.com/dotnet/performance/sessions/5a3db2a2-dd61-41bc-9847-4083906d08a5

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix code for all comments in review thread Fix review comments for CSV string.Split benchmarks Apr 11, 2026
Copilot AI requested a review from danmoseley April 11, 2026 22:52
@danmoseley
Copy link
Copy Markdown
Member

@copilot push to the original pr we don't need this new one

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 12, 2026

@copilot push to the original pr we don't need this new one

Unfortunately I'm only able to push to the branch of the PR I was invoked on (copilot/fix-comments-in-review-thread). I don't have the ability to push directly to pranas-string-split. You could cherry-pick the relevant commits from this branch onto the original PR, or the changes here could be referenced when merging.

@danmoseley
Copy link
Copy Markdown
Member

Closing: review fixes have been cherry-picked onto PR #4659 (branch pranas-string-split).

@danmoseley danmoseley closed this Apr 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants