do not cache function sets #16

Merged
merged 5 commits into from Jan 16, 2013

Conversation

Projects
None yet
2 participants
@Raynos
Contributor

Raynos commented Jan 16, 2013

Sets defined by filter functions may be impure. For example

function makeSet(foo) {
  return doc.createSet(function (s) { return s.foo === foo })
}

So it's not safe to cache them by their key

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Jan 16, 2013

Owner

please explain in more detail, what is the problem, and how do you fix it?

Owner

dominictarr commented Jan 16, 2013

please explain in more detail, what is the problem, and how do you fix it?

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Jan 16, 2013

Contributor

The problem is that if you create multiple sets then each time you create a new one crdt checks whether it has it already.

The problem is that with filter functions there is no notion of "has it already" where as with createSet("key", "value") there is. With the latter it's save to just return the set because it contains the same thing.

WIth a filter function it's not save to cache or return a cached set. This is because there is no way to translate a function into the set of items it contains. A pure function could be translated but an impure function can't be cached.

The way the current implementation in this PR fixes this issue is by simply not caching sets which are defined by functions

Contributor

Raynos commented Jan 16, 2013

The problem is that if you create multiple sets then each time you create a new one crdt checks whether it has it already.

The problem is that with filter functions there is no notion of "has it already" where as with createSet("key", "value") there is. With the latter it's save to just return the set because it contains the same thing.

WIth a filter function it's not save to cache or return a cached set. This is because there is no way to translate a function into the set of items it contains. A pure function could be translated but an impure function can't be cached.

The way the current implementation in this PR fixes this issue is by simply not caching sets which are defined by functions

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Jan 16, 2013

Contributor

Also whitespace changes everywhere because my text editor hates your text editor :/

Contributor

Raynos commented Jan 16, 2013

Also whitespace changes everywhere because my text editor hates your text editor :/

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Jan 16, 2013

Owner

hmm, by impure function you mean a function with side effects? or a function that has free variables that change (due to scope) or all of the above?

Owner

dominictarr commented Jan 16, 2013

hmm, by impure function you mean a function with side effects? or a function that has free variables that change (due to scope) or all of the above?

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Jan 16, 2013

Contributor

@dominictarr impure means "when given same inputs as arguments to function does not produce same outputs".

Which means it has state basically.

The fact that it may cause a side effect but always return same output is an example of an impure function that's safe to cache but that doesn't matter because it's not safe to cache all functions.

Contributor

Raynos commented Jan 16, 2013

@dominictarr impure means "when given same inputs as arguments to function does not produce same outputs".

Which means it has state basically.

The fact that it may cause a side effect but always return same output is an example of an impure function that's safe to cache but that doesn't matter because it's not safe to cache all functions.

@dominictarr dominictarr merged commit 18c22d7 into dominictarr:master Jan 16, 2013

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Jan 16, 2013

Owner

thanks, merged into 3.3.1

Owner

dominictarr commented Jan 16, 2013

thanks, merged into 3.3.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment