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

Unload issue with relationships #3296

Closed
igorT opened this issue Jun 11, 2015 · 3 comments
Closed

Unload issue with relationships #3296

igorT opened this issue Jun 11, 2015 · 3 comments

Comments

@igorT
Copy link
Member

igorT commented Jun 11, 2015

It is not clear what unloading should do with regards to relationships. It seems to me that it should dematerialize a record, but keep the internalModel intact if it has SSOT foreign key references(removing foreign keys seems obviously incorrect).
This is problematic though, because internalModels will still leak memory. How do we let users unload a bunch of related records? Does this mean we have to write a mini Garbage Collector, that will anytime you unload a record, check the foreign key dependencies and unload things it can. This doesn't sound too bad, can you confirm it is the right thing to do?

@wycats
Copy link
Member

wycats commented Jun 11, 2015

@igorT This seems acceptable. Do you mean something more than refcounting? Do you end up needing a cycle collector?

@igorT
Copy link
Member Author

igorT commented Jun 11, 2015

The algorithm needed to be implemented is:

  1. store.unloadRecord only dematerializes the record (internalModel.dematerialize()) which destroys the DS.Model
  2. We treat the record and relationships as a graph. Starting at the record do a breadth first search through the relationships, and see if you can reach a record that has not been dematerialized. If you can reach a live record, do nothing.
  3. If the graph is fully dematerialized, go and unload(destroy internalModel, remove from identity map) all reachable records including the current one

@Kilowhisky
Copy link

I can't quite wrap my head around this issue but would a symptom of this problem be that unloaded records are still being returned from store.findAll under some circumstances? I have a heavily interlinked model that i'm trying to unload and it seems to get stuck in an odd place where the record cannot be returned from a peekRecord but can be returned from a peekAll or findAll.

hjdivad added a commit to hjdivad/data that referenced this issue Oct 19, 2016
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Oct 19, 2016
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Oct 19, 2016
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Oct 21, 2016
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Oct 22, 2016
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Oct 22, 2016
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Oct 22, 2016
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Oct 22, 2016
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Oct 22, 2016
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Includes work from @igorT and @sly7-7

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Jan 15, 2017
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Finally
  - Add a lot of `toString`s to test classes, as a debugging convenience.
  - Minor test cleanup

Includes work from @igorT, @sly7-7 and @pangratz

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Jan 15, 2017
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Finally
  - Add a lot of `toString`s to test classes, as a debugging convenience.
  - Minor test cleanup

Includes work from @igorT, @sly7-7 and @pangratz

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Jan 15, 2017
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Finally
  - Add a lot of `toString`s to test classes, as a debugging convenience.
  - Minor test cleanup

Includes work from @igorT, @sly7-7 and @pangratz

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Jan 16, 2017
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Finally
  - Add a lot of `toString`s to test classes, as a debugging convenience.
  - Minor test cleanup

Includes work from @igorT, @sly7-7 and @pangratz

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Jan 16, 2017
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Finally
  - Add a lot of `toString`s to test classes, as a debugging convenience.
  - Minor test cleanup

Includes work from @igorT, @sly7-7 and @pangratz

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Jan 16, 2017
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Finally
  - Add a lot of `toString`s to test classes, as a debugging convenience.
  - Minor test cleanup

Includes work from @igorT, @sly7-7 and @pangratz

[Fix emberjs#3296]
[Supercede emberjs#3301]
hjdivad added a commit to hjdivad/data that referenced this issue Jan 16, 2017
`internalModel`s need to stay around as long as they're connected to a live
record via relationships.  Once an entire subgraph is removed however, we can
free all of it.

Also add a couple of "late private" fields to `internalModel`'s constructor for
shape preservation.

Finally
  - Add a lot of `toString`s to test classes, as a debugging convenience.
  - Minor test cleanup

Includes work from @igorT, @sly7-7 and @pangratz

[Fix emberjs#3296]
[Supercede emberjs#3301]
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

No branches or pull requests

3 participants