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

Form Class is loading queries (too?) eagerly causing (unnecessary) performance bottleneck #3746

Open
bnomei opened this issue Oct 3, 2021 · 4 comments
Labels
needs: delay ⏳️ Requires more time, on hold type: performance ⚡️ Is about performance; improves performance

Comments

@bnomei
Copy link

bnomei commented Oct 3, 2021

Describe the bug
Form class loads data even if it may be never used. it dataset is very big this causes a hugh delay for on each request. This is a common scenario in both autoid and boost plugin and currently make adding pages beyond 20k index impossible within 30sec timeout.

$form = Form::for($page, [

To Reproduce

  1. create a lot of pages of template human

  2. create a collection that does return a site->index()

<?php

return function () {
    return site()->index()->filterBy('template', 'human');
};
  1. create a page blueprint with a field with a multiselect query
# other fields
human:
  type: multiselect
  options: query
  max: 1
  query:
    fetch: kirby.collection('humans') # this does a site index -> filter
    text: "{{ page.title }}"
    value: "{{ page.id }}"
  1. create a single or a batch of new human page programmatically.
  2. the query will be executed in php code even it is most likely only used in api/kql/panel.

Expected behavior
the query should only be executed in context of api/kql/panel or when called explicitly in php.

Kirby Version
3.5.7 and 3.6.0-beta.1

bnomei added a commit to bnomei/kirby3-boost that referenced this issue Oct 3, 2021
bnomei added a commit to bnomei/kirby that referenced this issue Nov 18, 2021
Great performance gain as explained in issue.
closes getkirby#3746
bnomei added a commit to bnomei/kirby that referenced this issue Nov 18, 2021
…ons.


This does greatly improve performance when options are fetched. See issue for more information.

closes getkirby#3746
@stale
Copy link

stale bot commented Aug 10, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the type: stale 💤 Will be closed soon because there was no recent activity label Aug 10, 2022
@bnomei
Copy link
Author

bnomei commented Aug 11, 2022

dear bot. this issue has not been resolved. please keep it open.

@stale stale bot removed the type: stale 💤 Will be closed soon because there was no recent activity label Aug 11, 2022
@lukasbestle
Copy link
Member

I think we should revisit what you wrote in "expected behavior": If we can find a way to resolve the options less often and only when they are needed, this will increase performance by a lot, no matter if a cache is used or not.

@distantnative distantnative added the needs: delay ⏳️ Requires more time, on hold label Sep 20, 2022
@distantnative
Copy link
Member

We have started refactoring our blueprint and form classes. Would be best to tackle this, once that's done.

@github-actions github-actions bot added the type: stale 💤 Will be closed soon because there was no recent activity label Mar 20, 2023
@distantnative distantnative removed the type: stale 💤 Will be closed soon because there was no recent activity label Mar 20, 2023
@getkirby getkirby deleted a comment from github-actions bot Mar 20, 2023
@distantnative distantnative added the type: performance ⚡️ Is about performance; improves performance label Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: delay ⏳️ Requires more time, on hold type: performance ⚡️ Is about performance; improves performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants