Skip to content

Conversation

@jfmengels
Copy link

This improves the performance of the filter function.

The main change is that we don't call Result.map predicate. Let's look at the JS representation once compiled:

// Original version
var $author$project$Main$filter = F3(
	function (err, predicate, result) {
		var _v0 = A2($elm$core$Result$map, predicate, result);
		if (_v0.$ === 'Ok') {
			if (_v0.a) {
				return result;
			} else {
				return $elm$core$Result$Err(err);
			}
		} else {
			return result;
		}
	});

// Proposed
var $author$project$Main$filter = F3(
	function (err, predicate, result) {
		if (result.$ === 'Ok') {
			var a = result.a;
			return predicate(a) ? result : $elm$core$Result$Err(err);
		} else {
			return result;
		}
	});

In both versions, the function checks whether a result is Ok or not. In the proposed version we do not call Result.map when we don't need to (if Result is Err), and this removes an additional pattern matching and re-wrapping in Ok that are done in Result.map. In both cases, there is less work done that in the original version.

Here is a benchmark: https://ellie-app.com/fCC677f4YLKa1
Screenshot 2021-10-20 at 12 08 47

I have also tried a different version of the function which is available and benchmarked here but, the I find the benchmarks inconclusive compared to the original version, but they're definitely worse than the version from this PR.

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.

1 participant