Skip to content

Commit

Permalink
maybe fixed memoize to not iterate too eagerly (closes #191)
Browse files Browse the repository at this point in the history
  • Loading branch information
dtao committed Feb 2, 2017
1 parent d113176 commit b1640b7
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 8 deletions.
100 changes: 92 additions & 8 deletions lazy.js
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,10 @@
* @constructor
*/
function MemoizedSequence(parent) {
this.parent = parent;
this.parent = parent;
this.memo = [];
this.iterator = undefined;
this.complete = false;
}

// MemoizedSequence needs to have its prototype set up after ArrayLikeSequence
Expand Down Expand Up @@ -1931,6 +1934,38 @@
});
};

/**
* @constructor
*/
function Memoizer(memo, iterator) {
this.iterator = iterator;
this.memo = memo;
this.currentIndex = 0;
this.currentValue = undefined;
}

Memoizer.prototype.current = function current() {
return this.currentValue;
};

Memoizer.prototype.moveNext = function moveNext() {
var iterator = this.iterator,
memo = this.memo,
current;

if (this.currentIndex < memo.length) {
this.currentValue = memo[this.currentIndex++];
return true;
}

if (iterator.moveNext()) {
this.currentValue = memo[this.currentIndex++] = iterator.current();
return true;
}

return false;
};

/**
* @constructor
*/
Expand Down Expand Up @@ -3275,26 +3310,75 @@
// Now that we've fully initialized the ArrayLikeSequence prototype, we can
// set the prototype for MemoizedSequence.

MemoizedSequence.prototype = new ArrayLikeSequence();
MemoizedSequence.prototype = new Sequence();

MemoizedSequence.prototype.cache = function cache() {
return this.cachedResult || (this.cachedResult = this.parent.toArray());
MemoizedSequence.prototype.getParentIterator = function getParentIterator() {
// Since the premise of this sequence is that it only iterates over each
// element of its parent a grand total of one (1) time, we should only ever
// need to get the parent iterator once.
if (!this.iterator) {
this.iterator = this.parent.getIterator();
}

return this.iterator;
};

MemoizedSequence.prototype.getIterator = function getIterator() {
return new Memoizer(this.memo, this.getParentIterator());
};

MemoizedSequence.prototype.iterateTo = function iterateTo(i) {
var memo = this.memo,
iterator = this.getParentIterator();

while (i >= memo.length) {
if (!iterator.moveNext()) {
this.complete = true;
return false;
}

memo.push(iterator.current());
}

return true;
};

MemoizedSequence.prototype.get = function get(i) {
return this.cache()[i];
var memo = this.memo;

if (i < memo.length) {
return memo[i];
}

if (!this.iterateTo(i)) {
return undefined;
}

return memo[i];
};

MemoizedSequence.prototype.length = function length() {
return this.cache().length;
if (!this.complete) {
this.iterateTo(Infinity);
}

return this.memo.length;
};

MemoizedSequence.prototype.slice = function slice(begin, end) {
return this.cache().slice(begin, end);
if (!this.complete) {
this.iterateTo(end);
}

return Lazy(this.memo.slice(begin, end));
};

MemoizedSequence.prototype.toArray = function toArray() {
return this.cache().slice(0);
if (!this.complete) {
this.iterateTo(Infinity);
}

return this.memo.slice(0);
};

/**
Expand Down
43 changes: 43 additions & 0 deletions spec/memoize_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
describe('memoize', function() {
it('only iterates as far as it needs', function() {
var visited = [];

var sequence = Lazy([1, 2, 3, 4, 5]).map(function(x) {
visited.push(x);
return x;
});

var memoized = sequence.memoize();

expect(memoized.take(3)).toComprise([1, 2, 3]);
expect(visited).toEqual([1, 2, 3]);

// Ensure iterating over memoized again doesn't re-trigger side effects.
visited.length = 0;
expect(memoized.take(2)).toComprise([1, 2]);
expect(visited).toEqual([]);

// Reading one more from the sequence should result in 1 more side effect.
visited.length = 0;
expect(memoized.take(4)).toComprise([1, 2, 3, 4]);
expect(visited).toEqual([4]);

// Make sure actually iterating to the end doesn't cause any issues.
visited.length = 0;
expect(memoized).toComprise([1, 2, 3, 4, 5]);
expect(visited).toEqual([5]);

// Just for funsies.
visited.length = 0;
expect(memoized.map(Lazy.identity)).toComprise([1, 2, 3, 4, 5]);
expect(visited).toEqual([]);
});

it('works just fine on empty sequences', function() {
expect(Lazy([]).memoize()).toComprise([]);
});

it('works just fine on generated sequences', function() {
var numbers = Lazy.generate(Math.random).memoize().take(3);
});
});
1 change: 1 addition & 0 deletions spec/node_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ require("./sum_spec.js");
require("./watch_spec.js");
require("./merge_spec.js");
require("./join_spec.js");
require("./memoize_spec.js");

if (util.isHarmonySupported()) {
require('./es6_spec.js');
Expand Down

0 comments on commit b1640b7

Please sign in to comment.