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

GC-Free group iterator #3362

Merged
merged 1 commit into from
Feb 12, 2025
Merged

Conversation

Sword352
Copy link
Contributor

@Sword352 Sword352 commented Feb 11, 2025

This pull request inlines all of the methods from FlxTypedGroupIterator to ensure there are no object allocation when looping through a group.
This is a common practice taken by Haxe developers, and it's quite good hygiene to reduce GC pressure notably if you are looping through a group each frames.

I tried this change with the following code:

var group = new FlxGroup();
var memberId = 0;

for (member in group)
{
	member.ID = memberId++;
}

Here are the following JS and C++ output without this change:

var group = new flixel_group_FlxTypedGroup();
var memberId = 0;
var member = new flixel_group_FlxTypedGroupIterator(group.members,null);
while(member.hasNext()) {
	var member1 = member.next();
	member1.ID = memberId++;
}

cpp:

::flixel::group::FlxTypedGroup group =  ::flixel::group::FlxTypedGroup_obj::__alloc( HX_CTX ,null());
int memberId = 0;
{
	::Dynamic filter = null();
	::flixel::group::FlxTypedGroupIterator member =  ::flixel::group::FlxTypedGroupIterator_obj::__alloc( HX_CTX ,group->members,filter);
	while(member->hasNext()){
		::flixel::FlxBasic member1 = member->next().StaticCast<  ::flixel::FlxBasic >();
		memberId = (memberId + 1);
		member1->ID = (memberId - 1);
    }
}

(NOTE: for the C++ output I removed the HXLINE calls as it took a lot of place.)

Here is the JS output with this change (the C++ output is a bit long, but I can send it if required):

var group = new flixel_group_FlxTypedGroup();
var memberId = 0;
var _g__groupMembers = group.members;
var _g__filter = null;
var _g__cursor = 0;
var _g__length = _g__groupMembers.length;
while(true) {
	while(_g__cursor < _g__length && (_g__groupMembers[_g__cursor] == null || _g__filter != null && !_g__filter(_g__groupMembers[_g__cursor]))) ++_g__cursor;
	if(!(_g__cursor < _g__length)) {
		break;
	}
	while(_g__cursor < _g__length && (_g__groupMembers[_g__cursor] == null || _g__filter != null && !_g__filter(_g__groupMembers[_g__cursor]))) ++_g__cursor;
	var member = _g__cursor < _g__length ? _g__groupMembers[_g__cursor++] : null;
	member.ID = memberId++;
}

As you can see, the iterator has been fully inlined and no longer causes it to be allocated.

swordcube added a commit to swordcubes-grave-of-shite/flixel that referenced this pull request Feb 11, 2025
@Geokureli Geokureli added this to the Next Patch milestone Feb 12, 2025
@Geokureli Geokureli merged commit 14385b3 into HaxeFlixel:dev Feb 12, 2025
11 checks passed
@Geokureli
Copy link
Member

Geokureli commented Feb 12, 2025

Thanks, can't believe we forgot to inline those

@Sword352 Sword352 deleted the gc-free-group-iterator branch February 12, 2025 05:09
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.

2 participants