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

Allocation free Map.iterator() and Map.keys() #8806

Conversation

RealyUniqueName
Copy link
Member

@RealyUniqueName RealyUniqueName commented Sep 13, 2019

Ok, not completely free on all targets (e.g. we still need an array for keys on js), but close to that :)

Closes #3147

This branch contains only JS implementation for Map.iterator and Map.key to illustrate the idea.
The idea is to move iterator and key to @:using.
The same approach could be applied to Map.keyValueIterator().

class Main {
	static function main() {
		var m = [1 => 2, 3 => 4];
		for(i in m) {
			trace(i);
		}
	}
}
Generated JS with current development
// Generated by Haxe 4.0.0-rc.5
(function ($global) { "use strict";
var HxOverrides = function() { };
HxOverrides.iter = function(a) {
	return { cur : 0, arr : a, hasNext : function() {
		return this.cur < this.arr.length;
	}, next : function() {
		return this.arr[this.cur++];
	}};
};
var Main = function() { };
Main.main = function() {
	var _g = new haxe_ds_IntMap();
	_g.h[1] = 2;
	_g.h[3] = 4;
	var i = _g.iterator();
	while(i.hasNext()) console.log("src/Main.hx:5:",i.next());
};
var haxe_ds_IntMap = function() {
	this.h = { };
};
haxe_ds_IntMap.prototype = {
	keys: function() {
		var a = [];
		for( var key in this.h ) this.h.hasOwnProperty(key) ? a.push(key | 0) : null;
		return HxOverrides.iter(a);
	}
	,iterator: function() {
		return { ref : this.h, it : this.keys(), hasNext : function() {
			return this.it.hasNext();
		}, next : function() {
			var i = this.it.next();
			return this.ref[i];
		}};
	}
};
Main.main();
})({});
Generated JS with this PR
// Generated by Haxe 4.0.0-rc.5
(function ($global) { "use strict";
var Main = function() { };
Main.main = function() {
	var _g1_map_h = { };
	_g1_map_h[1] = 2;
	_g1_map_h[3] = 4;
	var a = [];
	for( var key in _g1_map_h ) _g1_map_h.hasOwnProperty(key) ? a.push(key | 0) : null;
	var keys = a;
	var _g1_keys = keys;
	var _g1_index = 0;
	var _g1_count = keys.length;
	while(_g1_index < _g1_count) console.log("src/Main.hx:5:",_g1_map_h[_g1_keys[_g1_index++]]);
};
Main.main();
})({});

@Gama11
Copy link
Member

Gama11 commented Sep 13, 2019

Relevant issue: #3147

@RealyUniqueName
Copy link
Member Author

For clarity:
With current development that sample allocates an array for keys, an iterator for keys, an iterator for values.
With this PR that sample only allocates an array for keys.

@RealyUniqueName RealyUniqueName added this to the Release 4.1 milestone Sep 13, 2019
@nadako
Copy link
Member

nadako commented Sep 13, 2019

Is this a (temporary?) workaround for #7379? I don't like that we have to go the weird way for such a basic thing...

@skial skial mentioned this pull request Sep 14, 2019
1 task
@RealyUniqueName
Copy link
Member Author

I think for iterating @:multiType we don't have an alternative currently. The best we can do is replace @:multiType entirely with a better feature.

@Simn
Copy link
Member

Simn commented Feb 17, 2020

I don't have a strong opinion on this one, so I'll let the two of you fight this out.

@Simn Simn removed their request for review February 17, 2020 08:46
@RealyUniqueName
Copy link
Member Author

I'd like to finish and merge this PR, because I doubt we will get an alternative for @:multiType any time soon.

@nadako
Copy link
Member

nadako commented May 24, 2020

I don't like this and I'm not sure why I'm assigned to it. As far as I understand something like #9432 will fix the original issue, so IMO it's better to go that way.

@nadako nadako removed their assignment May 24, 2020
basro added a commit to basro/haxe that referenced this pull request Jul 1, 2020
@RealyUniqueName RealyUniqueName modified the milestones: Release 4.2, Backlog Dec 16, 2020
@Simn Simn removed this from the Backlog milestone Mar 24, 2023
@Simn
Copy link
Member

Simn commented Mar 25, 2023

Looking at this again and considering all the possible problems, it's better to not do this. #7379 is still open to track the original problem.

@Simn Simn closed this Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Map iteration is excessively slow
4 participants