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

TextField High Memory Usage When Incrementing Large Numbers #2662

Open
RapperGF opened this issue Sep 23, 2023 · 16 comments
Open

TextField High Memory Usage When Incrementing Large Numbers #2662

RapperGF opened this issue Sep 23, 2023 · 16 comments

Comments

@RapperGF
Copy link

TextField Memory Leak on CPP and HL

Problem : On native targets openfl TextField allocates memory as the text increments this is also exposed on haxe flixel FlxText because it extends openfl TextField.

Reproducing: Creating a simple flixel app and adding a textfield that displays an incrementing integer will show it both at the flixel level or in an openfl textfield.

Author: 47rooks

import flixel.FlxState;
import flixel.text.FlxText;
import openfl.text.TextField;
import openfl.text.TextFormat;

class PlayState extends FlxState
{
    var _textF:FlxText;
    var _textStr:String;
    var _myInt:Int;
    var _firstDone:Bool;
    var _oflText:TextField;

    override public function create()
    {
        super.create();

        _myInt = 0; // Counter to increment
        _textStr = Std.string(_myInt);
        _textF = new FlxText(20, 100, 200, _textStr, 20);
        add(_textF);

        _oflText = new openfl.text.TextField();
        _oflText.selectable = false;
        _oflText.text = _textStr;
        _oflText.defaultTextFormat = new TextFormat(null, 14, 0xffffff);
        FlxG.game.addChild(_oflText);
    }

    @:access(flixel.system.frontEnds.BitmapFrontEnd)
    override public function update(elapsed:Float)
    {
        super.update(elapsed);

        _myInt++;

        // Uncomment for a flixel level test
        // _textF.text = Std.string(_myInt);

        // Check cache size
        var numEntries = 0;
        for (k in FlxG.bitmap._cache.keys())
        {
            numEntries++;
        }
        trace('FlxG.bitmap._cache numEntries=${numEntries}');

        // Update the OpenFL text - comment out to run the flixel only test```
        _oflText.text = Std.string(_myInt);
    }
}```

Openfl Ver: 9.2.x

Expected Behavior: 
TextField should not leak memory as it increments within a value. 

Affected Targets: CPP, HL. 

Additional context: Also occurs on Haxe Flixel with FlxText. 
@47rooks
Copy link

47rooks commented Sep 23, 2023

Here is a pure openfl test case:


import openfl.display.Sprite;
import openfl.events.Event;
import openfl.text.TextField;
import openfl.text.TextFormat;

class Main extends Sprite
{
	var _oflText:TextField;
	var _myInt:Int;
	
	public function new()
	{
		super();

		_oflText = new openfl.text.TextField();
		_oflText.selectable = false;
		_oflText.text = Std.string(_myInt);
		_oflText.defaultTextFormat = new TextFormat(null, 36, 0x000000);
		_oflText.cacheAsBitmap = false;
		_oflText.x = 200;
		_oflText.y = 200;
		addChild(_oflText);

		this.addEventListener(Event.ENTER_FRAME, everyFrame);
	}

	function everyFrame(_):Void {
		_myInt++;
		_oflText.text = Std.string(_myInt);
	}
}

Tested with :

haxelib list
hxcpp-debug-server: [1.2.4]
hxcpp: [4.3.2]
lime: [8.0.2]
openfl: [9.2.2]

Use an OS utility to monitor the memory. It will increase, and may at times appearing stable but then begin increasing again.

@Dimensionscape
Copy link
Member

Dimensionscape commented Sep 23, 2023

This is normal behavior due to the shape cache.

Increased memory usage is the tradeoff for vastly improved cpu performance on large volumes of text. The cache should only keep new shape combinations, but there may be a more memory efficient way to address it.

When cleaning up the text field, the cache should be freed up for collection and therefore it should not be considered a leak.

@Dimensionscape
Copy link
Member

Dimensionscape commented Sep 23, 2023

After considering this some more, I think it might make sense to only use the cache after a certain number of word breaks to avoid adverse affects of using text fields to display a counter.

Or perhaps just clean up the cache on empty textfields.

I need to think about this more.

Just how bad does the memory usage peak?

I believe a counter to 1,000,000 should only use a maximum of 6 mb. If the counter is reused, it will never exceed this. A counter to 10,000,000 will use 70 mb if every number between 1 and 10,000,000 is tapped. It does get exponentially worse so I can see how it might become worrisome for some use cases.

@RapperGF
Copy link
Author

After considering this some more, I think it might make sense to only use the cache after a certain number of word breaks to avoid adverse affects of using text fields to display a counter.

Or perhaps just clean up the cache on empty textfields.

I need to think about this more.

Just how bad does the memory usage peak?

I believe a counter to 1,000,000 should only use a maximum of 6 mb. If the counter is reused, it will never exceed this. A counter to 10,000,000 will use 70 mb if every number between 1 and 10,000,000 is tapped. It does get exponentially worse so I can see how it might become worrisome for some use cases.

The memory proceeds to count up past 600 megabytes of ram when the digits reach a threshold of 10 or 12. This is within a timeframe of 15 minutes the problem is worse on flixel based applications because memory problems cause performance altercations which leads to framerate dips. Small digits have no issue but if you use a line of text with score, lives, etc this quickly becomes worrysome.

@Dimensionscape
Copy link
Member

Yea, I didn't consider the implications of a large number counter.

The shape cache allows us to quickly append or change text without having to calculate the positions of the entire text field repeatedly. It also helps when multiples of the same word are used in the same text field because it already has the cached measurement ready for that word. This was crucial to allow large dynamic textfields to function without severe drops in frame rate when typing or changing text programmatically.

It does even have some minor benefit for small dynamic text fields because the measurement calculations are expensive, however it should not come at the cost of exuberant memory usage even if a 10+ digit counter is kind of rare use case.

The solution is kind of tricky since resetting the cache on the text setter will void out any benefit of the cache to begin with. The cache is absolutely necessary or many general use cases for text field simply become unusable at scale.

Will think of something compromising.

@RapperGF
Copy link
Author

Yea, I didn't consider the implications of a large number counter.

The shape cache allows us to quickly append or change text without having to calculate the positions of the entire text field repeatedly. It also helps when multiples of the same word are used in the same text field because it already has the cached measurement ready for that word. This was crucial to allow large dynamic textfields to function without severe drops in frame rate when typing or changing text programmatically.

It does even have some minor benefit for small dynamic text fields because the measurement calculations are expensive, however it should not come at the cost of exuberant memory usage even if a 10+ digit counter is kind of rare use case.

The solution is kind of tricky since resetting the cache on the text setter will void out any benefit of the cache to begin with. The cache is absolutely necessary or many general use cases for text field simply become unusable at scale.

Will think of something compromising.

Thinking of making like TextCounterField or some way to manually clear the memory buildup? You'd think if the text changes it would deallocate the prev text.

@Dimensionscape
Copy link
Member

The problem is actually due to the way TextEngine internals work. Essentially, we should only be caching individual glyphs which is both memory efficient and cpu efficient, but OpenFL instead processes word breaks in their entirety.

In order to properly solve the problem, a large part of the text internals need to be rewritten. Text rendering and layout is fundamentally very complicated and there are many pitfalls. This is definitely on the todo list, but it is also going to be a long and difficult process.

In the interim, we can either provide a flag to turn the cache off entirely in the application or just avoid caching text fields with only a few word breaks.

@RapperGF
Copy link
Author

RapperGF commented Sep 24, 2023

The problem is actually due to the way TextEngine internals work. Essentially, we should only be caching individual glyphs which is both memory efficient and cpu efficient, but OpenFL instead processes word breaks in their entirety.

In order to properly solve the problem, a large part of the text internals need to be rewritten. Text rendering and layout is fundamentally very complicated and there are many pitfalls. This is definitely on the todo list, but it is also going to be a long and difficult process.

In the interim, we can either provide a flag to turn the cache off entirely in the application or just avoid caching text fields with only a few word breaks.

Okay! As long as like text like 'Score: 120000 | Combo: 3000 | Accuracy: 100%' does not leak memory when incrementing these values as one line than a solution flag like that could be beneficial!!

@Dimensionscape
Copy link
Member

Currently, if you use JUSTIFY alignment it will not use the cache at all. I thought I would point that out.

@Dimensionscape
Copy link
Member

Dimensionscape commented Sep 24, 2023

The problem is actually due to the way TextEngine internals work. Essentially, we should only be caching individual glyphs which is both memory efficient and cpu efficient, but OpenFL instead processes word breaks in their entirety.

In order to properly solve the problem, a large part of the text internals need to be rewritten. Text rendering and layout is fundamentally very complicated and there are many pitfalls. This is definitely on the todo list, but it is also going to be a long and difficult process.

In the interim, we can either provide a flag to turn the cache off entirely in the application or just avoid caching text fields with only a few word breaks.

Okay! As long as like text like 'Score: 120000 | Combo: 3000 | Accuracy: 100%' does not leak memory when incrementing these values as one line than a solution flag like that could be beneficial!!

Without the cache I would recommend splitting this up into multiple text fields since the cpu usage in changing the textfield will likely cause frame rate instability if it changes frequently.

See, when you alter the text field, without the cache it loops through the text word by word in order to measure it. Every time. Longer text fields means more iterations. (O(n) complexity). The cache gives us near O(1) meaning we only have to measure new words.

I assume the author of the original internals thought measuring entire words would be higher performing rather than processing each glyph position independently, which, without a cache is probably true. If we only cache glyph measurements then we can take advantage of both memory and cpu optimization.

@Rudyrue
Copy link

Rudyrue commented Sep 24, 2023

Currently, if you use JUSTIFY alignment it will not use the cache at all. I thought I would point that out.

what does justify do other than not using the cache i don't use openfl stuff that much

@Dimensionscape
Copy link
Member

Currently, if you use JUSTIFY alignment it will not use the cache at all. I thought I would point that out.

what does justify do other than not using the cache i don't use openfl stuff that much

Justify alignment is a text alignment option used in typesetting, word processing, and graphic design. When text is justified, it means that the spaces between words are adjusted to make the text align evenly along both the left and right margins of a text block. In other words, the text is stretched or compressed as needed to ensure that each line of text extends from the left edge to the right edge of its containing area.

It is the same in Flixel:
https://api.haxeflixel.com/flixel/text/FlxTextAlign.html#JUSTIFY

@Dimensionscape
Copy link
Member

A workaround for now could be to use multiple text fields inside a single container.

TextField(Score:) TextFieldWithJustify(120000) TextField(| Combo:) TextFieldWithJustify(3000) TextField(| Accuracy:) TextFieldWithJustify(100%)

Now, due to the nature of justify you also have to workaround the spacing by padding with 0's so score would default be 000000(max digits) combo would be 0000 and accuracy would be 000%.

It might not be ideal, but I just wanted to show that you can work around this if you're creative. I realize you shouldn't have to. I'll try to provide a solid solution before the next haxelib release.

@RapperGF
Copy link
Author

A workaround for now could be to use multiple text fields inside a single container.

TextField(Score:) TextFieldWithJustify(120000) TextField(| Combo:) TextFieldWithJustify(3000) TextField(| Accuracy:) TextFieldWithJustify(100%)

Now, due to the nature of justify you also have to workaround the spacing by padding with 0's so score would default be 000000(max digits) combo would be 0000 and accuracy would be 000%.

It might not be ideal, but I just wanted to show that you can work around this if you're creative. I realize you shouldn't have to. I'll try to provide a solid solution before the next haxelib release.

I just tested JUSTIFY and the memory builds HOWEVER it does not stay built up, once it does an initial cache it seems to slowly increase then decrease averaging at the same memory it began its first cache at. It is no longer a LEAK but the memory does change this is viable as a temporary solution. I think for a future endeavor TextField could be changed to have minimal memory buildup without utilizing JUSTIFY as alignment ;3

@Dimensionscape
Copy link
Member

Justify simply doesnt use the cache because it breaks and is rarely used for long dynamic text.

I just pushed a change allowing you to turn off the cache entirely to the dev branch using <haxedef name="skip_measurement_cache"/>.

Keep in mind that this define is likely to be removed in the future as we continue to improve TextEngine internals.

@Dimensionscape
Copy link
Member

Dimensionscape commented Sep 24, 2023

Also keep in mind that this was never a leak.

A memory leak is a situation in computer programming where a program or application fails to release memory (RAM) that it has allocated but no longer needs. In other words, memory that was allocated for a particular task or data storage is not properly deallocated or freed when it is no longer in use.

In this case, if you dispose of the textfield, it will deallocate the memory. The cache is there for a reason. It is not a leak...

Hope we can develop a more permanent fix soon.

Cheers!

@Dimensionscape Dimensionscape changed the title TextField Memory Leak on CPP and HL TextField High Memory Usage When Incrementing Large Numbers Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants