CacheEntryHelper.Current uses expensiveAsyncLocal. Before accessing it, we can just check if the options can be propagated and don't use it at all if not.

+7.5% (+18k) RPS for TechEmpower benchmark!

load	before	after
CPU Usage (%)	19	21
Cores usage (%)	528	574
Working Set (MB)	48	48
Build Time (ms)	4,500	4,541
Start Time (ms)	0	0
Published Size (KB)	76,401	76,401
.NET Core SDK Version	3.1.404	3.1.404
First Request (ms)	76	78
Requests/sec	234,029	252,468
Requests	3,533,646	3,812,090
Mean latency (ms)	1.28	1.18
Max latency (ms)	61.30	47.33
Bad responses	0	0
Socket errors	0	0
Read throughput (MB/s)	736.49	794.52
Latency 50th (ms)	1.06	0.99
Latency 75th (ms)	1.17	1.09
Latency 90th (ms)	1.33	1.23
Latency 99th (ms)	10.08	9.10

inline the expiration check hot paths

load	before	after
CPU Usage (%)	21	21
Cores usage (%)	574	580
Working Set (MB)	48	48
Build Time (ms)	4,541	4,923
Start Time (ms)	0	0
Published Size (KB)	76,401	76,401
.NET Core SDK Version	3.1.404	3.1.404
First Request (ms)	78	79
Requests/sec	252,468	259,239
Requests	3,812,090	3,914,333
Mean latency (ms)	1.18	1.15
Max latency (ms)	47.33	48.63
Bad responses	0	0
Socket errors	0	0
Read throughput (MB/s)	794.52	815.83
Latency 50th (ms)	0.99	0.96
Latency 75th (ms)	1.09	1.07
Latency 90th (ms)	1.23	1.19
Latency 99th (ms)	9.10	9.03

don't acces expensive CacheEntryHelper.Current if options can't be pr…

fe723e0

…opagated anyway, +20k RPS for TechEmpower benchmark!

adamsitnik added tenet-performance

Performance related issue

area-Extensions-Caching labels

Nov 27, 2020

adamsitnik added this to the6.0.0 milestone

Nov 27, 2020

adamsitnik requested review fromeerhardt andmaryamariyan

November 27, 2020 16:31

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

How much does the ‘in’ help here?

Copy link

MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I just wanted to be 100% sure that this struct is not being copied on Linux.

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I just wanted to be 100% sure that this struct is not being copied on Linux.

Did this one change make any measurable impact? We should not be sprinkling lots ofins everywhere unless they're actually meaningful.

eerhardt reviewed

Nov 27, 2020

View reviewed changes

src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs Outdated


		if(_slidingExpiration.HasValue
		&&(now-LastAccessed)>=_slidingExpiration)
		returnSlowPath(now);

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Is the idea here that the above check can get inlined, and then the “SlowPath” code remains a normal method call? Is there a good guidance on when this pattern should be used?

Side nit - is “SlowPath” a good name for this method?

Copy link

MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Is the idea here that the above check can get inlined, and then the “SlowPath” code remains a normal method call?

Yes, exactly.

Is there a good guidance on when this pattern should be used?

I would say that it should be used when the condtion typically returns false and rarely executes the code inside it (like throwing exceptions) and is ofc on a hot path (small helper methods executed frequently)