Use new Dictionary AlternateLookup to avoid allocation in DictionaryJumpTable

[andrewjsaid]

Use new Dictionary AlternateLookup to avoid allocation in DictionaryJumpTable

Description

Simple change which removes the substring allocation in DictionaryJumpTable using the GetAlternateLookup<ReadOnlySpan<char>> feature of .NET 9.

With this change all JumpTable implementations can accept ReadOnlySpan<char> as a parameter instead of string, PathSegment. I'd be happy to include that change if it's wanted.

[BrennanConroy]

Awesome, was looking forward to aspnetcore adopting the new alternate lookup feature.

Could you run the micro-benchmarks before and after this change?
https://github.com/dotnet/aspnetcore/blob/main/src/Http/Routing/perf/Microbenchmarks/Matching/JumpTableMultipleEntryBenchmark.cs

[andrewjsaid]

Interestingly enough the PR looks to be slightly slower (5ns -> 11ns) with the benchmark as-is whereas I would have expected no difference.

However there's a flaw with the benchmark in that it currently always compares the entire string which is unusual and which negates the benefit proposed in this PR. In most real-world cases the PathSegment is only part of the string and so substrings are allocated.

I've modified the benchmark to account for this by using only part of the path string. The modified benchmark results are below. Overall less significant than I would have liked but it's faster and allocates less memory but only if we accept PathSegment usually points to a substring of Path. [edit: I confirmed that this is always the case since all paths start with /]

Before:

Method	Count	Mean	Error	StdDev	Op/s	Ratio	RatioSD	Gen 0	Gen 1	Gen 2	Allocated
Baseline	2	0.6304 ns	0.0052 ns	0.0049 ns	1,586,354,557.5	1.00	0.00	-	-	-	-
Dictionary	2	13.9512 ns	0.1030 ns	0.0860 ns	71,678,284.5	22.13	0.29	0.0011	-	-	82 B
Baseline	5	0.6195 ns	0.0067 ns	0.0060 ns	1,614,331,173.5	1.00	0.00	-	-	-	-
Dictionary	5	14.5809 ns	0.1890 ns	0.1578 ns	68,582,745.6	23.55	0.32	0.0011	-	-	82 B
Baseline	10	0.6169 ns	0.0111 ns	0.0104 ns	1,620,884,005.6	1.00	0.00	-	-	-	-
Dictionary	10	15.2295 ns	0.2681 ns	0.3486 ns	65,661,888.5	24.69	0.86	0.0011	-	-	82 B
Baseline	25	0.6165 ns	0.0060 ns	0.0056 ns	1,621,971,184.8	1.00	0.00	-	-	-	-
Dictionary	25	16.9301 ns	0.2896 ns	0.2568 ns	59,066,511.0	27.47	0.46	0.0011	-	-	82 B
Baseline	50	0.6172 ns	0.0060 ns	0.0056 ns	1,620,306,284.5	1.00	0.00	-	-	-	-
Dictionary	50	18.1510 ns	0.0577 ns	0.0482 ns	55,093,297.9	29.42	0.34	0.0011	-	-	82 B
Baseline	100	0.6176 ns	0.0070 ns	0.0066 ns	1,619,194,748.0	1.00	0.00	-	-	-	-
Dictionary	100	20.4224 ns	0.1325 ns	0.1174 ns	48,965,762.2	33.04	0.38	0.0011	-	-	82 B

After:

Method	Count	Mean	Error	StdDev	Op/s	Ratio	RatioSD	Gen 0	Gen 1	Gen 2	Allocated
Baseline	2	0.6283 ns	0.0088 ns	0.0082 ns	1,591,566,799.9	1.00	0.00	-	-	-	-
Dictionary	2	11.8230 ns	0.1413 ns	0.1321 ns	84,580,838.3	18.82	0.22	-	-	-	-
Baseline	5	0.6149 ns	0.0052 ns	0.0046 ns	1,626,151,413.8	1.00	0.00	-	-	-	-
Dictionary	5	11.8060 ns	0.1567 ns	0.1466 ns	84,702,576.7	19.19	0.33	-	-	-	-
Baseline	10	0.6141 ns	0.0067 ns	0.0063 ns	1,628,406,229.3	1.00	0.00	-	-	-	-
Dictionary	10	12.0957 ns	0.1984 ns	0.1856 ns	82,673,853.8	19.70	0.39	-	-	-	-
Baseline	25	0.6163 ns	0.0044 ns	0.0039 ns	1,622,602,556.1	1.00	0.00	-	-	-	-
Dictionary	25	13.0903 ns	0.2482 ns	0.2200 ns	76,392,361.3	21.24	0.34	-	-	-	-
Baseline	50	0.6159 ns	0.0095 ns	0.0089 ns	1,623,584,223.2	1.00	0.00	-	-	-	-
Dictionary	50	14.6091 ns	0.0292 ns	0.0273 ns	68,450,281.6	23.72	0.35	-	-	-	-
Baseline	100	0.6143 ns	0.0078 ns	0.0073 ns	1,627,848,658.0	1.00	0.00	-	-	-	-
Dictionary	100	16.1624 ns	0.0569 ns	0.0475 ns	61,871,831.2	26.28	0.34	-	-	-	-

[BrennanConroy]

Shouldn't this be - 1?

[andrewjsaid]

I attempted to trim one from the start and one from the end but I don't suppose it matters too much as long as it's a substring.

[andrewjsaid]

I've created dotnet/runtime#108732 which if accepted/merged should further improve the performance of the AlternateLookup used in this PR.

[BrennanConroy]

/azp run

Was waiting for the runtime PR so we could see the full perf gains. But since it's taking time to get a review we can just merge this since it's still goodness.

[azure-pipelines]

Command 'run

Was' is not supported by Azure Pipelines.



Supported commands

• help:
• Get descriptions, examples and documentation about supported commands
• Example: help "command_name"
• list:
• List all pipelines for this repository using a comment.
• Example: "list"
• run:
• Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
• Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
• where:
• Report back the Azure DevOps orgs that are related to this repository and org
• Example: "where"

See additional documentation.

[BrennanConroy]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).