October: File model query is very slow with large data

[mabehiry]

The file model query is very slow. This is an example of a query used, it takes about 580 ms to execute. I'm viewing about 100 images in the page, so it takes a long time for the page to render.

SELECT *
FROM system_files
WHERE system_files.attachment_type = 'Me\Articles\Models\Article'
AND system_files.attachment_id = '235017'
AND field = 'image'
AND system_files.attachment_id IS NOT NULL
ORDER BY sort_order ASC
LIMIT 1

The table has about 220,000 records.

Steps To Reproduce:

Insert a lot of records to a model and attach files to each one. Then try to get around 100 record with files in a page.
This is how I call the attachment:

echo $model->image->getPath();

Suggestions:

I suggest these actions:
1- To remove this check:
AND system_files.attachment_id IS NOT NULL

2- And to remove this also:
ORDER BY sort_order ASC

3- And to make this field int instead of varchar:
attachment_id

[w20k]

@mabehiry why not to add an index? Pretty sure it should solve your performance issue.

[mabehiry]

@mabehiry why not to add an index? Pretty sure it should solve your performance issue.

There are already 3 indexes:
field
attachment_id
attachment_type

I added sort_order index also

[Teranode]

sometimes attachment_id isn't an integer (for instance I use uuids instead of ids)

I completely agree that ORDER BY sort_order ASC should be removed, it slows it down significantly, so instead i had to manually create the query without the sort order (on in some cases create my own sort after its been queried)

[w20k]

@Teranode could you provide a code analyze why it's generated, I think it's something related to the QueryBuilder.

[Teranode]

It uses the sortable trait:
https://github.com/octobercms/library/blob/develop/src/Database/Traits/Sortable.php

So what "could" be done is to set a variable for if you want sorting to be enabled from the trait, while also keeping the sortable train there so if you need to use it in the future you can just set the variable for sort to true when defining the relationship

[mabehiry]

When I run the query in MySql console, it takes only 0.0032 seconds. But in the CMS it takes 0.637 seconds.

This is the query:
select * from system_files where system_files.attachment_type = 'Sunsoft\Articles\Models\Article' and system_files.attachment_id = '143555' and field = 'image' and system_files.attachment_id is not null order by sort_order asc limit 1

And attached here the screenshot of query in CMS and in SQL console (the CMS debug is by Bedard.Debugbar)

[w20k]

@mabehiry Pure SQL will always be faster, no doubt!

[bennothommo]

I think a better optimisation would be to retrieve all the attached images in one query as opposed to doing a single query for each file. Even if the query is slow, it would be one slow query and not 100.

[w20k]

@bennothommo I was thinking the same, but my concern is to write tests, before breaking things 😄

[bennothommo]

@mabehiry Are you using an attachOne file association for this scenario?

[mabehiry]

@bennothommo Yes I'm using attachOne

[bennothommo]

@mabehiry Thanks, I presume then that what you have set up is a page where you can view 100 articles in a single page, and each one has a single file upload for an image or some other file, is that correct?

If so, and if you want to avoid the loading delay right now, you could use the mediafinder widget instead of a fileupload widget to pick the image, as the mediafinder widget saves the URL of the media item into the database as a string, instead of using a separate model. This would drop the extra SQL query for each file upload.

[mabehiry]

@bennothommo Yes, it's a good idea. Thank you. But you know, when using fileupload I can crop the image and use various sizes. I use it for the thumb images.
Anyway, I used caching for images so that I don't need to call it every time. But I think if this is a better way to optimize SQL query, it will be good.
Thank you

[Teranode]

@LukeTowers I noticed that the SortableScope has this already:
https://github.com/octobercms/library/blob/develop/src/Database/SortableScope.php#L20

Is that how it's suppose to be in that method?

[Samuell1]

1- To remove this check:
AND system_files.attachment_id IS NOT NULL

This cant be removed or it will break relations. Tested it and if you remove that check it will automatically attach other file that is null. It needs to be tested more to be removed from AttachOneOrMany https://github.com/octobercms/library/blob/develop/src/Database/Relations/AttachOneOrMany.php#L48

@mabehiry do you think you can test it out if removing that it speedups query?

@bennothommo I think better will be better if there is option to cache file relations

[github-actions[bot]]

This issue will be closed and archived in 3 days, as there has been no activity in the last 30 days. If this issue is still relevant or you would like to see action on it, please respond and we will get the ball rolling.

[Samuell1]

This should be reopen

[bennothommo]

@Samuell1 Is this being worked on?

[w20k]

Don't think that anyone is dealing with it currently.

[Samuell1]

@bennothommo i was trying to somehow optimalize relations like i said in comment before but it needs to be more tested.

[bennothommo]

@Samuell1 no worries, I have re-opened it pending your PR.

[Samuell1]

@bennothommo thanks!

Another thing is that why file relation has sortable trait added on it that is maybe second big performance hit.