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

"runner.fastReruns" experimental feature #4628

Merged
merged 12 commits into from
Apr 21, 2022
Merged

Conversation

tconkling
Copy link
Contributor

@tconkling tconkling commented Apr 20, 2022

"fast reruns"

  • fastReruns is a new opt-in Streamlit feature that makes interactive Streamlit apps much more responsive. (In internal tests we've seen up to 100X response time improvements using the feature.)
  • When the fastReruns is enabled, a rerun request will immediately spin up a new ScriptRunner to handle the rerun, without waiting for the current ScriptRunner to stop. (With fastReruns disabled, Streamlit behaves as it does now: a rerun (or stop) request will be handled at the script's next "yield" point - that is, when it calls any st.foo function.)
  • "config.fastReruns" is the new config option for controlling this feature. It's defaulted to False for now - we'll spend some time with it before defaulting it to True.

Risks

SessionState race conditions

An app using the faster-reruns option will spin up a new ScriptRunner to handle a rerun request even as its current ScriptRunner is in the process of stopping. When this happens, we want to avoid race conditions that might occur when both script executions are accessing the same SessionState data simultaneously.

SessionState access is now thread-safe, and a ScriptRunner that's in the process of stopping - but hasn't fully shut down - will have its SessionState interface "disconnected" from the underlying state. BUT: apps that mutate their underlying session state outside of statements like st.session_state["foo"] = 123 run the risk of data races if they use this feature.

For example, this would be a problem:

if 'values' not in st.session_state:
  st.session_state['values'] = [1, 2, 3]

values = st.session_state['values']
next_value = len(values)

# Mutation outside of a session_state statement.
# This is a race condition if fastReruns is enabled!
values.append(next_value)

The safe way to do this:

if 'values' not in st.session_state:
  st.session_state['values'] = [1, 2, 3]

# Create a copy of the object before mutating it
new_values = st.session_state['values'].copy()

# Mutate the object and then assign it back to session_state.
# (The assignment statement will be a no-op if this script
# is being shutdown due to a fastRerun.)
new_values.append(len(values))
st.session_state['values'] = new_values

More script execution threads!

With fastReruns enabled, Streamlit will be running more script execution threads simultaneously. With enough connected users spamming rerun requests, it's possible this could make an interactive app slower than it would be otherwise. If this becomes an issue, we should look into things like thread pools and throttles. (Maybe we temporarily "downgrade" an overloaded fastReruns=True Streamlit app to fastReruns=False while it catches its breath?)

Etc

This PR also adds some internal frontend benchmarking code (PerformanceEvents). It's disabled by default so won't have an impact on end users.

Super-basic recording and analysis of performance-related events that happen during a rerun

- `PerformanceEvents` is a simple static class that records typed, timestamped events
- `App` and `WebsocketConnection` log these events
- `RerunAnalyzer` generates a simple JSON report of the performance analysis for a single rerun

This may or may not be code that we end up shipping in Streamlit. If we do ship it, it'll need tests and maybe better documentation and a better way to enable and disable "performance logging" mode. But in the meantime, it'll be useful for measuring the impact of our "make streamlit reruns faster" work.
(Finally) implement opt-in fast reruns

- "config.fastReruns" is the new config option for controlling this feature. It's defaulted to `True` in this feature branch, but I think we'll probably default it to `False`  when this is merged to develop?
- When the option is enabled, a rerun request will cause an existing ScriptRunner to shutdown, and immediately spin up a new one to handle the rerun.
* develop:
  ForwardMsgQueue: no longer thread-safe (#4568)
* develop:
  Fix component-template e2e test failures (#4602)
  setup.py: handle latest Pipenv (2022.4.8) (#4598)
  Rename PageLayoutContext to AppContext (#4566)
  Fix st.bokeh_chart long description docstring (#4575)
* develop:
  Reduce SessionState API surface area (#4607)
* develop:
  SessionState: no longer a MutableMapping (#4612)
  Bump moment from 2.29.1 to 2.29.3 in /frontend (#4615)
* develop:
  Change names from *FileWatcher to *PathWatcher (#4604)
  Wait for first process to start up before running another process (#4618)
* develop:
  AutoSessionState -> SessionStateProxy (#4623)
### Motivation

An app using the faster-reruns option may spin up a new ScriptRunner to handle a rerun request even as its current ScriptRunner is in the process of stopping. When this happens, we want to avoid race conditions that might occur when both script executions are accessing the same SessionState data simultaneously.

### SafeSessionState

This PR introduces `SafeSessionState`, which is a thread-safe wrapper around `SessionState`. It has a `disconnect` function that's called by ScriptRunner when it gets a stop request. A "disconnected" SafeSessionState no longer reads and writes its underlying SessionState instance - all operations becomes no-ops (or raise KeyErrors, as appropriate).

This means that a script running inside a ScriptRunner that is shutting down will not mutate session state (or register widgets).

(I don't love the name "SafeSessionState", but I couldn't think of a better one! "DisconnectableSessionState" is maybe more accurate, but is too much of a mouthful?)

### Other minor bits

- `SessionState.clear_state()` -> `SessionState.clear()`
- `SessionState.as_widget_states()` -> `SessionState.get_widget_states()`
- `SessionState._keys()` is now private
- `validate_key()` -> `require_is_valid_user_key()`
* develop:
  Update TypeScript (non-JSX) test files to comply with type checker (#4626)
  Header and components UI adjustments (#4596)
  Teach EventBasedPathWatcher how to watch for directory changes (#4609)
  Add helpers to compute hashes for directories (#4608)
  Upgrade TypeScript and force run on jslint (#4620)
  Remove pydeck as a dependency (#4560)
@tconkling tconkling changed the title "runner.fastReruns" feature "runner.fastReruns" experimental feature Apr 21, 2022
@tconkling tconkling marked this pull request as ready for review April 21, 2022 17:50
@AnOctopus AnOctopus self-requested a review April 21, 2022 18:27
@tconkling tconkling merged commit 4d78c03 into develop Apr 21, 2022
@tconkling tconkling deleted the feature/faster-reruns branch June 24, 2022 15:23
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.

None yet

2 participants