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

Improve ReactPy Performance #557

Closed
Archmonger opened this issue Dec 26, 2021 · 6 comments · Fixed by #1165
Closed

Improve ReactPy Performance #557

Archmonger opened this issue Dec 26, 2021 · 6 comments · Fixed by #1165
Labels
priority-3-low May be resolved one any timeline. type-investigation About research and gathering information

Comments

@Archmonger
Copy link
Contributor

Archmonger commented Dec 26, 2021

Proposed Changes

Find the hot spots in ReactPy's code-base in order to identify where attention should be focused. Once we know this, we will be able to focus our efforts on performance improvements.

Implementation Details

Analysis

Use Yappi to profile ReactPy dispatchers and determine what is taking the most significant amounts of time.

We will also need to figure out how to test websocket concurrency. It seems like there are very few tools for this, especially for our applications. We're probably going to have to create out own benchmarking tools. Here's some resources we might want to read through later.

Sync and Async

Currently, ReactPy relies on synchronous code to properly queue renders. Sync code is known to blocks the asyncio event loop, which causes concurrency issues.

See section "Two Worlds of Python" for why this is a problem:
https://arunrocks.com/a-guide-to-asgi-in-django-30-and-its-performance/

All synchronous functions should be converted to async within ReactPy core. They could alternatively be threadpooled. See reactive-python/reactpy-django#31 for the original discussion on this.

Extreme Countermeasures

If optimizations can't be suitably performed within Python, consider writing C/C++ code exposed via Python APIs that performs the same functionality as the non-performant parts of IDOM.

@Archmonger Archmonger added the flag-triage Not prioritized. label Dec 26, 2021
@Archmonger
Copy link
Contributor Author

Archmonger commented Dec 26, 2021

A quick test of using pyjion.enable() within dispatch_single_view has shown performance degradation, likely due to the async with limitations noted in their documentation.

@rmorshea
Copy link
Collaborator

There's definitely a lot of room for improvement. The areas to focus on, probably in this order, are:

  1. Computing JSON patches. Right now this is using a pure Python library that I've come to notice isn't very actively maintained.
  2. Find and optimize the hotspots in rendering VDOM models (see idom/core/layout.py).
  3. Server-side rendering (i.e. serve the initial view as static HTML).

@Archmonger
Copy link
Contributor Author

Archmonger commented Dec 27, 2021

JSON Patch

I tried doing some research, seems like there isn't anything else worthwhile on the python side. This is a promising variant on NPM.

This raises a question of whether it's worth the performance to compute a JSON Patch in Python as opposed to resending the whole JSON content body (and calculating the patch client sided).

Internal Layout

Just during my digging I've already seen a couple of situations that could be migrated to list comprehension, which is significantly faster than python loops.

SEO Compatible SSR

Implementing SSR is definitely going to complicate IDOM core. SSR may actually reduce IDOM performance. Everything happening on the client side isn't blocking the WS queue.

This is a big topic to tackle and deserves a discussion on its own.

I'm not even sure where I'd say we add this to the roadmap. Priority of this card depends on long term goals and focus. I'd personally rank it pretty low on the list.

Roadmap

I'd like to work with you on fillling out GH Projects to provide visibility on version goals.

We should also tag up to discuss OSS funding. IDOM is definitely something that I'd be able to pitch.

@rmorshea
Copy link
Collaborator

Yeah, scrap what I said before, we need to profile before doing any work. It's very possible that using JSON patch doesn't provide any significant benefit in the most common use cases.

To the roadmap, yes let's find a time to coordinate that. There's actually a roadmap page in the docs I haven't update in a while.

And for funding, I suppose the question is what it would be for. I'm happy to get reimbursed for the time I already put in, but I don't have more to give, so adding money won't make things go faster. It would be more valuable to find a way to encourage outside contributions, either from yourself or others.

@Archmonger
Copy link
Contributor Author

Archmonger commented Jan 2, 2022

See reactive-python/reactpy-django#31 for a relevant performance issue that will eventually need to be resolved.

I believe it's going to be incredibly important for IDOM to support async components. In the case of Django, users will experience WS performance degradation due to blocking the websocket event loop.

@rmorshea rmorshea added type-feature About new capabilities and removed flag-triage Not prioritized. labels Jan 9, 2022
@rmorshea rmorshea added this to the 2.0 milestone Jan 11, 2022
@rmorshea rmorshea added priority-3-low May be resolved one any timeline. type-investigation About research and gathering information and removed type-feature About new capabilities labels Jan 13, 2022
@Archmonger Archmonger changed the title Improve Layout Rendering Performance Improve IDOM Performance Jan 31, 2022
@Archmonger Archmonger modified the milestones: Luxury, Essential Jan 29, 2023
@rmorshea rmorshea removed this from the Essential milestone Feb 21, 2023
@rmorshea rmorshea changed the title Improve IDOM Performance Improve ReactPy Performance Jun 22, 2023
@Archmonger
Copy link
Contributor Author

Archmonger commented Jul 10, 2023

We might get some performance benefit by pushing ReactPy into it's own dedicated backhaul thread (containing an asyncio loop). One thread per ASGI worker, which we can initialize within reactpy:__init__.py.

That way, rendering components won't prevent the webserver from doing webserver things.

@Archmonger Archmonger linked a pull request Nov 28, 2023 that will close this issue
2 tasks
@Archmonger Archmonger mentioned this issue Nov 28, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-3-low May be resolved one any timeline. type-investigation About research and gathering information
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants