- Issue created by @weseze
- π§πͺBelgium weseze
I did some more digging and found the issue is the "webform_library_info_build()".
This function loads all webform, checks them for custom CSS/JS and adds those as a library if needed.The website where we noticed the issue has 1000+ webforms. So trying to load them all leads to 200Mb of memory usage.
I tried changing the code to load them 1-by-1 and clear the static entity caching between each, but this didn't help. The memory usage stayed the same. I thought this would have been a quick and easy "fix"... :(
The better solution would be to split the code in 2 pieces:
1/ general CSS/JSS assets need to be added once in a "shared" library for all webforms
2/ only webforms that have specific CSS/JSS assets need to be loaded: this could still be an issue if there are many: not sure how to address that... - First commit to issue fork.
- π¬π§United Kingdom catch
The issue here is loading every webform and then looping over them to check if they have css or javascript.
Config entities support entity queries, so it's possible to pre-filter with an entity query. For me this reduced the time taken for this hook from five seconds to about 230ms - Merge request !656Issue #3497954: Excessive memory use when building dynamic library definitions β (Open) created by catch
- π¬π§United Kingdom catch
Double checked and ::exists() definitely isn't an option here - it results in loading all the webforms in, so I think the
<> ''
logic is correct here. - πΊπΈUnited States jrockowitz Brooklyn, NY
The problem and solution makes a lot of sense to me. I am good with it
- π¨π¦Canada Liam Morland Ontario, CA π¨π¦
There is an unresolved thread; @catch, @heddn
- π¬π§United Kingdom catch
I had a look into the test failures.
The problem is in the way that config entity queries work.
The key value entries look like this:
| config.entity.key_store.webform_options | uuid:eba38bf8-26e1-49ef-87f8-5093b78cf9f8 | a:1:{i:0;s:33:"webform.webform_options.education";}
webform.webform.test_cards_javascript.yml
has a very long string of JavaScript in it.The key_store tries to put the full value of the javascript string into 'name' which is not possible given it's varchar 128.
If we truncated this to varchar 128 in core it should work, so I will open an issue to discuss that. This might need to be postponed but would prefer to see feedback for the core issue is like first, there might be other ways to handle this if that specific change to core isn't doable.
- π¬π§United Kingdom catch
Opened π The config key_store (entity queries) can't handle long key values Active .
If that issue isn't fixable in a way that helps here, another possible approach would be to add 'has_js' and 'has_css' keys to webform config, obviously without a UI, then populate them based on the js/css keys - these would only need boolean values so would then fit in the config entity key_store and be queryable, and we could simplify the queries here.
I think this issue is severe enough that would be worth doing, but it would be a lot more work - would need an upgrade path etc.
- πΊπΈUnited States jrockowitz Brooklyn, NY
@catch Could we store which webforms has js/css libraries via state?
On webform create, update and delete, we can update the 'webform_libraries' state value.
In 'webform_libraries' state value we could store the webform ids or the entire library definition.
- π¬π§United Kingdom catch
@jrockowitz yeah that could be a good option!
Storing the entire library definition in state could be heavy now because state is cached with a cache collector, but could use key/value directly instead of state - this is only used when building library definitions so doesn't really need to be chained fast/state as such. But also if it's only a list of which webforms have libraries that would definitely be small enough for state.
Either way would be simpler than adding the new keys to the config entities.
- πΊπΈUnited States jrockowitz Brooklyn, NY
Please review the MR for storing webform libraries via state. The MR accounts for sites that have custom CSS/JS for all webforms.
We could move the global CSS/JS into a dedicated library attached seperately.
- π¬π§United Kingdom catch
We could move the global CSS/JS into a dedicated library attached seperately.
That sounds like a good idea to avoid OOM for sites that have configured global CSS/JS. I thought I had a way around this when I commented on the MR but if that global css/js currently has to be added to the individual webform library definitions then I don't think there's an alternative to splitting it into its own thing.
A separate library for that would also help with asset aggregate hit rates (e.g. one library for all webforms, instead of unique one for each webform but with the same actual CSS/JS in it) - at least if I'm understanding what the differences are - not actually using the feature anywhere that I know of.
- πΊπΈUnited States jrockowitz Brooklyn, NY
jrockowitz β changed the visibility of the branch 3497954-store-webform-with-css-js-in-state to hidden.