Consider caching of DisplayedString

[eXpl0it3r]

Currently, every call of DisplayedString does a marshalling from/to C and copying of the string data.
If you update the Text instance often (e.g. FPS counter), than we're wasting a lot of processing time to convert and allocate memory to get the string data.

It would be less expensive too cache the string data on C# side. As there isn't anything on SFML's side that could lead to a change of the string on its own, it should be fine to update the string data in the set method and have return only return the cached value.

Three (?) edge cases to consider:

• set has never been called
• Text instance was copied
• CPointer changes somehow

See also forum thread.

[charleyah]

Hey @eXpl0it3r

I gave this a read over and I encountered the same concern the user expressed on the forum some time ago with the Text Class. Regularly updating DisplayedString on hot path is always a fun one with the .NET GC, but it isn’t too surprising 😊

If you update the Text instance often (e.g. FPS counter), than we're wasting a lot of processing time to convert and allocate memory to get the string data.

I'm not too sure how caching will help reduce the overhead on setting DisplayedString as it will only save on reading the property. The setter still needs to allocate a new copy of the string with a null terminator in UTF32. Is there something I'm missing?

If a really efficient method of setting DisplayedString was desired, I'd be opting for a new method which works directly with reusable buffers rather than strings or a new Text type built around that purpose where string value isn't so static. I have provided at the end some further optimisations that could be applied to the current implementation though.

See below on handling the edge cases you listed.

Getting DisplayedString

• Cached value can be read from a field just like Front property already is.

• As DisplayedString is set by all constructors that create a new Text instance, the only time the setter won’t be called is if Text is copied. The copy constructor should set the cached field directly. SFML/CSFML will be copying the internal value so C# must do the same to keep values identical.

• The only time CPointer will change is when Text instance is disposed. Currently, getting DisplayedString will throw ObjectDisposedException if CPointer is invalid. Reading the cached field should behave the same way by checking if CPointer is valid before returning or throw.

Setting DisplayedString

• Update cached field with the new string value after the string value in CSFML/SFML C++ has been updated. Updating cached field after will ensure ObjectDisposedException is thrown first if CPointer is invalid.

If you’re happy with the above, I can put a pull request in for this. Also let me know if any of the below suggestions are worth considering.

Further Optimisations To Consider

• If the number of utf32 codepoints is small enough to not blow the stack space, stackalloc could be used to allocate the encoding buffer on stack instead of managed array from Encoding class. For example, stack allocation up to 512 bytes would skip allocating managed utf32 array for strings up to 128 codepoints including null terminator.

• Add null terminator to the encoded utf32 buffer instead of allocating a new copy of the string with a null terminator before encoding. Encoding.GetByteCount can be used to know ahead of time how big the buffer needs to be.

• As EnumerateRunes is not available in SFML.Net's case, Using Encoder class directly instead of Encoding class is also a option. This can remove allocations by Encoding.GetBytes for their internal FallbackBuffer as Encoder reuses the same FallbackBuffer instance at the cost of the caller having to reset it. This does require keeping track of a Encoder instance either in ThreadStatic or per Text instance though.

• The Forum user's suggestion of a reusable encoding buffer static or instance would also be beneficial. The best approach to this is if static is to use ArrayPool<T>.Shared for creating reusable arrays for short lived buffer space. ArrayPools are currently not available in SFML.Net due to .NET Standard version. Might be something worth revisiting after SFML 3 release.

[eXpl0it3r]

I'm not too sure how caching will help reduce the overhead on setting DisplayedString as it will only save on reading the property. The setter still needs to allocate a new copy of the string with a null terminator in UTF32. Is there something I'm missing?

This issue is IMHO mostly about the reading of the string. When we set the string data, we can't really get around passing the data to CSFML/SFML. Setting the string once per frame should be performant enough, even if it's not necessarily recommended. Unless you want some special text effect that needs a change every frame, it's better to build your own caching strategy to only update SFML.Text instances when you actually have a changed string.

If you’re happy with the above, I can put a pull request in for this. Also let me know if any of the below suggestions are worth considering.

Sounds reasonable to me.

Further Optimisations To Consider

Not sure these optimizations really offer any noticeable advantage. I rather keep the code straight forward, unless we can really determine a massive bottleneck.