graphistry / falcor

Graphistry forks of the FalcorJS family of projects in a lerna-powered mono-repo.
23 stars 3 forks source link

simultaneous requests against model w/ recycleJSON returns duplicate data #14

Closed jameslaneconkling closed 7 years ago

jameslaneconkling commented 7 years ago
const model = new Model({
  source: new DataSource(...),
  recycleJSON: true
});

model.get(['search', 'EORJy', 'match', 'length'])
  .subscribe({
    next(res) { console.log('search result:', res); }
  });

model.get(['thing', '1ZJya', 'defaultLabel'])
  .subscribe({
    next(res) { console.log('item result:', res); }
  });

// > "search result: { json: { search: EORJy: { ... } } }
// > "item result: { json: { search: EORJy: { ... } } }

The model behaves as expected when recycleJSON: true is removed. Also, this does not appear to occur when I change model.get to model.getValue, or make the second get request after the first has resolved.

trxcllnt commented 7 years ago

@jameslaneconkling ah yes, sorry for the confusion here. The recycleJSON flag is an optimization we can make assuming all the requests to the same model instance are roughly the same query structure, and (ideally) switchMap'd.

tl;dr to get you unblocked (or here's a longer example in esnextb.in):

let searchModel = model._clone({ _seed: {} });
let thingModel = model._clone({ _seed: {} });
searchModel.get(['search', 'EORJy', 'match', 'length'])
  .subscribe({
    next(res) { console.log('search result:', res); }
  });
thingModel.get(['thing', '1ZJya', 'defaultLabel'])
  .subscribe({
    next(res) { console.log('item result:', res); }
  });

Longer explanation:

Without recycleJSON turned on, every action on a falcor model (get, set, call) creates a brand new JSON object in which to write the results. This works great using falcor as vanilla JSON RPC library, but is prohibitively expensive if you want to make get calls as part of, for example, the React component lifecycle. recycleJSON is designed to optimize for the component lifecycle call patterns.

When a component receives new props (or context), the component plucks most recent parent falcor model from React's context, creates a deref'd clone for itself using the data value from its current or nextProps (data is expected to be a node from a parent's falcor request), then calls get for its paths on that cloned model (source here).

The deref'd Model stores this data object as its seed value, and reuses it across get requests. In recycleJSON mode, hashcode and version metadata are written to a private namespace for each JSON branch created by a request. The hashcode uniquely describes the paths from the request that have been materialized into the branch, and the version is the monotonically increasing version number of the data in the falcor cache.

This allows us to diff each request against the request that was made last time, and only walk bits of the request tree that have changed in the cache, or are not in the recycled seed. In the best (and most common) case with React, most requests are identical and nothing has changed in the cache, so the get() call is a noOp (this is why this test clocks in at 4MM ops/sec).

If a request is made that excludes branches or values that were previously requested, the excluded values are stripped out of the recycle'd JSON value (the behavior you're seeing here), so we don't leak memory.

jameslaneconkling commented 7 years ago

Ah, OK, makes sense.

For reference, if I'm looking for inspiration on react-falcor integration, should I be tracking falcor-react-redux or falcor-react-schema? My impression was that they both did the same thing, with one possibly superseding the other. (?)

trxcllnt commented 7 years ago

@jameslaneconkling yep, that's right. The falcor-react-redux lib assumes an implicit dependency on Redux paradigms, and provides conveniences for binding Redux "action creators" to the context of the container's current falcor model. Originally we'd envisioned a redux store that was a hybrid of the Falcor JSON tree and client-side-only redux state, but eventually abandoned that idea because it wasn't fast enough.

The falcor-react-schema project is meant to supersede falcor-react-redux. We have the falcor-react-redux lib in production today alongside falcor-react-schema, but only the falcor-router schema container, as migrating our view components away from redux is a chore we have yet to tackle.

I've been deep in GPU land lately, so I haven't had time to finish the tests. I'll work today/this weekend to get the tests shaped into an example project and demo that runs in esnextb.in