Improve performance for accessing files from IDOM_WEB_MODULES_DIR about reactpy-django HOT 10 CLOSED

Ah, but one could use the same component more than once, so we could skip registration entirely if it's already there.

Created an issue: #5

from reactpy-django.

Caching the register function with functools.cache would provide the same ability to skip re-registration, and would allow us to skip the sys.modules check. Could be slightly more performant than checking IDOM_REGISTERED_COMPONENTS.

What do you think?

from reactpy-django.

@Archmonger I'm looking at what's required to implement this and its kind of ugly stuff. I'd mostly be copy-pasting code I don't really understand which makes me nervous about maintaining it going forward. I'm also still a bit skeptical that this will actually be a bottle neck. Given that, I think I'm going to hold off for now. If it does prove to be a real issue, we can revisit at that point.

With that said, I think there are two simpler solutions to this problem of the dynamic files in IDOM_WEB_MODULES_DIR that can improve performance and will be easier to maintain:

1. Make the view reading these web modules async so it can await the file instead of blocking until the file system is available.
2. Allow users to configure a cross-process cache using Django's cache framework.

from reactpy-django.

Async

I agree that it should be async. Generally all functions and views that target Django 3.0+ should be async. That is, unless you're writing Django code that uses calls the DB (Django team is still working on async DB ORM).

HTML tag parsing

I also was having trouble following the django-compressor code, but I'll see if I can digest it to become maintainable.

If not, I do have another idea in mind to accomplish the same thing.

We could directly call all configured staticfile finders and, as you suggested before, do a regex match on all .html files for the idom component tag.

Bottleneck?

To my understanding, all JS Web Modules should become apparent on the idom component's first call. If that is indeed the case then I'm not seeing any edge cases of this implementation.

But, if for some reason IDOM does not deterministically collect all required web modules on first call, then we'd need to reasses.

Caching

I do support us caching the view. But, not all Django users run with any cache backends enabled, and additionally not all cache backends are multiprocessing compatible. I recall the only built-in that is multiprocessing compatible is memcached. There's a third party backend that's multiprocessing compatible but it's less frequently used.

So perhaps, use Django's django.views.decorators.cache.cache_page when the user has a cache backend enabled (settings.py:CACHE), and fallback to functools.cache otherwise.

from reactpy-django.

Bottleneck?

I'm also still a bit skeptical that this will actually be a bottle neck.

The this in that statement referred to accessing the JS files. That is, that accessing the JS files using the "dynamic" approach (as apposed to being able to load them from a static dir) would become a bottleneck.

For now, I think I'm going to make the function async, and add the basic caching with functools we'd already talked about. I'll leave this issue open, and mark with with the "future" label.

Since a static collection method would be a backwards compatible addition, I think I feel comfortable releasing with the aforementioned changes so I can move on to other things in idom core.

from reactpy-django.

If we do intend to implement this as a management command later, I've determined the relevant lines of code related to django_compressor. Although their code is severely lacking comments, I personally think it's fairly easy to follow.

See django-compressor/compress.py:80-233.

Django Compressor Psuedocode

1. Create a list of all templates via get_loaders().
2. Collect all non-broken templates into a list. Broken syntax is checked via jinja2 parse() or django parse().
3. Perform _compress_template on each templatetag via a ThreadPoolExecutor.
   • This notably uses django-compressor.offline.django.walk_nodes and django-compressor.offline.jinja2.walk_nodes to break down each template into a list of templatetags.
   • By default I believe they're collecting this info using Python's built-in HTMLParser (extensible to utilize lxml, beautifulsoup, or html5lib). We could probably replace this with regex.

Future Implementation

Since we're leaving this for later, we're going to need to follow in django-compressor's footsteps of having a COLLECT_OFFLINE setting for backwards compatibility.

from reactpy-django.

On the note of performance, we may also want to consider caching the output of _register_component. I'm not sure if that'll have any ramifications though.

from reactpy-django.

This check should cover that since we avoid attempting an import if it's already in the set of imported modules.

from reactpy-django.

Progress Update

A lot of this issue has been resolved.

List of a few things that have been developed

• Async web module view
• Web module caching
• Component tag parser inspired by django-compressor

Task remaining

• Management command for collecting static files ahead of time

from reactpy-django.

I'm going to close this issue based on the title.

The fully async web module fetching we have developed is good enough for now.

Will create a separate issue specific to a new collect management command.

from reactpy-django.