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

Async hist loading #287

Merged
merged 24 commits into from
Mar 2, 2022
Merged

Async hist loading #287

merged 24 commits into from
Mar 2, 2022

Conversation

goodboy
Copy link
Contributor

@goodboy goodboy commented Feb 28, 2022

Re-work of the piker.data.feed machinery to do async loading of both real-time and historical data for faster chart startup and a better setup to get the work from #93 landed.

Part of this work discovered the unexpected behavior that is python-trio/trio#2258.

Included are a bunch of other changes summarized by "topic":


miscellaneous fixups

  • docs strings added to curve graphics code
  • changed display loop throttle warnings to only fire on >= display rate cycles
  • dropped stale deps on msgpack and msgpack-numpy
  • place the sample period (aka "time frame") in the window header
  • add an open_piker_runtime() to make it easy to write a client that uses all the tractor apis correctly

data feed rework

  • drop passing ShmArrays to the backend stream_quotes() endpoints since they don't manually write themselves
  • allow using open_feed() without requiring a real-time quote stream (eg. just for accessing shm / history)
  • drop the dedicated task to update graphics on sample period steps, instead just let the display loop detect new samples and update on the next event
  • expect fsps to request the sample period for incrementing shm history based on calculating the step size from source data
  • don't crash (just warn) on a double remove of a sampled fee subscription

misc graphics

  • make ChartPlotWidget.increment_view() take a steps: int
  • don't zero clearing rate fsp curves on a new sample step
  • fix the display loop's mamin() range calcer to not crash on no-data-in-view

Break up real-time quote feed and history loading into 2 separate tasks
and deliver a client side `data.Feed` as soon as history is loaded
(instead of waiting for a rt quote - the previous logic). If
a symbol doesn't have history then likely the feed shouldn't be loaded
(since presumably client code will need at least "some" datums history
to do anything) and waiting on a real-time quote is dumb, since it'll
hang if the market isn't open XD. If a symbol doesn't have history we
can always write a zero/null array when we run into that case. This also
greatly speeds up feed loading when both history and quotes are available.

TL;DR summary:
- add a `_Feedsbus.start_task()` one-cancel-scope-per-task method for
  assisting with (re-)starting and stopping long running persistent
  feeds (basically a "one cancels one" style nursery API).
- add a `manage_history()` task which does all history loading (and
  eventually real-time writing) which has an independent signal and
  start it in a separate task.
- drop the "sample rate per symbol" stuff since client code doesn't really
  care when it can just inspect shm indexing/time-steps itself.
- run throttle tasks in the bus nursery thus avoiding cancelling the
  underlying sampler task on feed client disconnects.
- don't store a repeated ref the bus nursery's cancel scope..
Since moving to a "god loop" for graphics, we don't really need to have
a dedicated task for updating graphics on new sample increments. The
only UX difference will be that curves won't be updated until an actual new
rt-quote-event triggers the graphics loop -> so we'll have the chart
"jump" to a new position and new curve segments generated only when new
data arrives. This is imo fine since it's just less "idle" updates
where the chart would sit printing the same (last) value every step.
Instead only update the view increment if a new index is detected by
reading shm.

If we ever want this dedicated task update again this commit can be
easily reverted B)
@@ -447,3 +494,25 @@ async def maybe_open_emsd(

) as portal:
yield portal


# TODO: ideally we can start the tsdb "on demand" but it's
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obvs coming down the road as part of what results from #247, left in as a guide.

# are diffed on each draw cycle anyway; so updates to the
# "curve" length is already automatic.

# increment the view position by the sample offset.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is less costly then a task context switch and gives a more "lazy" UX then the below left dedicated "update task" approach.

@@ -381,14 +380,19 @@ async def poll_and_sync_to_step(

s, step, ld = is_synced(src, dst)

# detect sample period step for subscription to increment
# signal
times = src.array['time']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll probably want a standard api for this eventually?

trio.CancelScope] = trio.TASK_STATUS_IGNORED,
) -> None:
with trio.CancelScope() as cs:
self.nursery.start_soon(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should probably move this to be a self.nusery.start() for now until python-trio/trio#2258 is resolved.

'''
# TODO: history retreival, see if we can pull from an existing
# ``marketstored`` daemon
log.info('Scanning for existing `marketstored`')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops, this should probably get commented.
Obviously these imports will come in with #247.


if opened:
if arrays:
await tractor.breakpoint()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just placeholder for #247 work.

@goodboy goodboy merged commit 5fb85d9 into master Mar 2, 2022
@goodboy goodboy deleted the async_hist_loading branch March 2, 2022 15:04
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.

2 participants