Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add caching to DrawShapedText #3033

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MichaelRumpler
Copy link

Description of Change

This enables a user to set a cache duration. When set to a value > 0, it caches the SKShaper and the SKShaper.Result thus repeated calls with the same font and/or text are faster.

Bugs Fixed

API Changes

Added:

  • public static void SetShaperCacheDuration(this SKCanvas canvas, uint milliseconds)

Behavioral Changes

If the cache duration was set to a value bigger than zero, DrawShapedText will cache the SKShaper and the SKShaper.Result for that amount of milliseconds. Therefore repeated calls with the same SKFont and/or string will be faster.

If the cache duration is set to 0, then the cache will be cleared, all contained SKShaper objects disposed and caching will be disabled.

The default cache duration is 0. So if SetShaperCacheDuration is never called, DrawShapedText works as before.

Required skia PR

None.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Merged related skia PRs
  • Changes adhere to coding standard
  • Updated documentation

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@taublast
Copy link
Contributor

Very nice one!!

Possible to avoid ConcurrentDictionary? IMHO it affects very badly animations scenarios, triggering GC due to its allocation mechanics, thus creating messy lag spikes during animations.
Once one starts a gaming loop that might come into play.

Maybe add a lock to a Dictionary.. Just thinking.

@mattleibow
Copy link
Contributor

Is it possible to use System.Runtime.Caching.MemoryCache?

https://learn.microsoft.com/dotnet/api/system.runtime.caching.memorycache

Not sure if this is a good thing or not? Do people use this API? Is this sufficient?

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@MichaelRumpler
Copy link
Author

I didn't want to introduce another dependency. But I can try it and see what the benchmarks say.

A Dictionary with lock was a bit faster than the ConcurrentDictionary, but not very much. What I actually planned, but didn't find the time to is to look at the source of System.Runtime.Caching and see if I can do something similar.

@MichaelRumpler
Copy link
Author

I tested System.Runtime.Caching.MemoryCache but it has several drawbacks:

  • it uses a string as key
  • it stores an object which must be boxed

Both make it slower than the Dictionary<int, Tuple>.

Here are the benchmarks with the cache variants:

Method Mean Error StdDev Gen0 Gen1 Allocated
DrawShapedAscii 134.396 us 0.4493 us 0.4203 us 0.2441 - 3144 B
DrawAsciiWithShaper 19.046 us 0.1644 us 0.1283 us 0.3052 - 2744 B
DrawAsciiWithConcurrentCache 9.845 us 0.0486 us 0.0455 us 0.0916 0.0763 848 B
DrawAsciiWithDictionaryCache 9.575 us 0.0355 us 0.0315 us 0.0763 0.0610 752 B
DrawAsciiWithRuntimeCache 10.934 us 0.0564 us 0.0528 us 0.1831 0.1526 1560 B
DrawShapedLigatures 191.784 us 1.3686 us 1.2133 us 0.2441 - 3144 B
DrawLigaturesWithShaper 72.422 us 0.5261 us 0.4921 us 0.2441 - 2744 B
DrawLigaturesWithConcurrentCache 8.561 us 0.0280 us 0.0262 us 0.0916 0.0763 848 B
DrawLigaturesWithDictionaryCache 8.372 us 0.0513 us 0.0480 us 0.0763 0.0610 752 B
DrawLigaturesWithRuntimeCache 10.120 us 0.0264 us 0.0206 us 0.7019 0.6714 6072 B
DrawShapedEmojis 62.447 us 0.2296 us 0.2035 us 0.1221 - 1888 B
DrawEmojisWithShaper 17.385 us 0.0405 us 0.0338 us 0.1526 - 1488 B
DrawEmojisWithConcurrentCache 13.649 us 0.2212 us 0.2069 us 0.0916 0.0763 848 B
DrawEmojisWithDictionaryCache 13.553 us 0.1254 us 0.1173 us 0.0763 0.0610 752 B
DrawEmojisWithRuntimeCache 15.427 us 0.2149 us 0.2010 us 0.7019 0.6714 6120 B
DrawShapedRandom 125.474 us 1.1728 us 1.0397 us - - 1943 B
DrawRandomWithShaper 10.477 us 0.0267 us 0.0209 us 0.1831 - 1543 B
DrawRandomWithConcurrentCache 7.811 us 0.0747 us 0.0662 us 0.1068 0.0992 911 B
DrawRandomWithDictionaryCache 7.538 us 0.0872 us 0.0773 us 0.0916 0.0763 815 B
DrawRandomWithRuntimeCache 9.752 us 0.1542 us 0.1367 us 0.1831 0.1678 1582 B

The project with the benchmarks for the various variants is on https://github.com/MichaelRumpler/SkiaTextRendering.

As you can see the simple Dictionary with a lock was always fastest with the least amount of memory used. So I changed it to this now.

@nd1012
Copy link

nd1012 commented Nov 6, 2024

the simple Dictionary with a lock was always fastest

In comparsion with the ConcurrentDictionary this is only true, as long as you don't use the specialized concurrent access methods like GetOrAdd, TryGetValue, TryUpdate and TryRemove. If you'd implement them using a simple lock statement, the ConcurrentDictionary implementation would be way faster. I can tell this, because I've implemented it just a few days ago, and benchmarks showed me a huge performance difference - so I removed my implementation with lock, finally, because 5.000us (Dictionary with lock) vs 79us (ConcurrentDictionary) during heavy multithreading doesn't really need a discussion - but for that result I've used the ConcurrentDictionary different.

Anyway these are my tests results when comparing different thread-safe dictionary access methods using max. 8 threads:

Method Mean Error StdDev Allocated
Dictionary 2.959 ms 0.0086 ms 0.0076 ms 4.03 KB
ConcurrentDictionary 15.351 ms 0.2132 ms 0.1994 ms 4.74 KB
ConcurrentLockDictionary 2.954 ms 0.0322 ms 0.0301 ms 4.05 KB
DictionaryManyItems 20.729 ms 0.1421 ms 0.1329 ms 3.8 KB
ConcurrentDictionaryManyItems 10.981 ms 0.0424 ms 0.0376 ms 3.73 KB
ConcurrentLockDictionaryManyItems 23.529 ms 0.3235 ms 0.3026 ms 3.81 KB
DictionaryManyItemsRnd 53.207 ms 1.0308 ms 1.6048 ms 4 KB
ConcurrentDictionaryManyItemsRnd 14.295 ms 0.0320 ms 0.0299 ms 3.87 KB
ConcurrentLockDictionaryManyItemsRnd 55.961 ms 1.1031 ms 1.8731 ms 3.98 KB

Dictionary is being used with a simple lock just as you do. ConcurrentDictionary is the .NET implementation using AddOrUpdate, and finally ConcurrentLockDictionary is my implementation of the ConcurrentDictionary interface using an underlying Dictionary with lock. The first 3 tests only work with a single item, while the ManyItems tests contain ushort.MaxValue << 3 items, which are being updated sequential using Parallel.For. The tests ending with ManyItemsRnd use random access. The ManyItems* tests are where the ConcurrentDictionary can gain most points because of its data and access locking management.

However, it depends on the ConcurrentDictionary usage, if it has advantages or not. But anyway, a ConcurrentDictionary uses way more memory than a Dictionary does (which is not visible in the test results 'cause I use static instances).

Btw. find my tests here, if you want.

When it's only about a few items, Dictionary with lock should be fine - otherwise ConcurrentDictionary may outperform.

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

Comment on lines +136 to +140
shaper = shaperCache.TryGetValue(key, out var value)
? value.shaper
: new SKShaper(typeface);

shaperCache[key] = (shaper, DateTime.Now); // update timestamp
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

I don't know how GetOrAdd could help, but I tried AddOrUpdate here:

var newTuple = shaperCache.AddOrUpdate(key,
	_ => (new SKShaper(typeface), DateTime.Now),
	(_, old) => (old.shaper, DateTime.Now));

But the simple Dictionary with a lock is still faster.

lock (shaperCache)
{
	shaper = shaperCache.TryGetValue(key, out var value)
		? value.shaper
		: new SKShaper(typeface);

	shaperCache[key] = (shaper, DateTime.Now);      // update timestamp
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add caching in DrawShapedText
4 participants