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

Merge SetUIElementRequests on the backend #1374

Merged
merged 9 commits into from
May 15, 2024

Conversation

akshayka
Copy link
Contributor

@akshayka akshayka commented May 14, 2024

This PR moves merging of SetUIElementValueRequest to the backend, eliminating the need model the runtime state machine in the frontend. As a result this PR also eliminates the FE mechanism of registering run starts and run ends, which was error prone.

In the backend, this is accomplished by adding a new queue for SetUIElementValueRequests -- these requests are stored in both the control queue, which determines ordering of SetUIElementValueRequests relative to other types of control quests, and this new queue. The new queue is just used for batching.

An alternative would be to split the control queue into two queues, or even N queues, one for each type of request, and then add a new routing queue that gave the order in which the queues should be pulled from. But that was more work, and kind of overkill ... memory pressure due to copying the SetUIElementValueRequest is negligible.

Copy link

vercel bot commented May 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marimo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 15, 2024 0:56am
marimo-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 15, 2024 0:56am

@mscolnick mscolnick merged commit f5e51df into main May 15, 2024
28 of 29 checks passed
@mscolnick mscolnick deleted the aka/improve-merge-set-ui-elems branch May 15, 2024 12:58
Copy link

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.5.3-dev11

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