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

page medias: bad performances and bad practices in admin #788

Closed
maximelebreton opened this issue Sep 19, 2016 · 15 comments
Closed

page medias: bad performances and bad practices in admin #788

maximelebreton opened this issue Sep 19, 2016 · 15 comments
Assignees

Comments

@maximelebreton
Copy link

maximelebreton commented Sep 19, 2016

I have a custom page, with two PageMediaSelect per List field.
And with only four items based on the same projects.json, it takes about 25s to load* all the PageMediaSelect fields, because the same request is called 9 times, and these requests aren't cached...

I'm surprised about the lack of good practices** with all that concerns 'Ajax' & Performance (#779, #741), because these are basic things, and Grav provides many more complex scenarios with success!
So maybe the admin performance part needs a serious investigation and refactoring?

*on a PlanetHoster mutualised server

**store JSON results in a variable or in local storage, & PHP Caching and Content Retrieval Function

ps: I try to make constructive feedback, I love Grav, and your amazing job guys


image


image

@w00fz
Copy link
Member

w00fz commented Sep 19, 2016

You shouldn't be using PageMediaSelect, that is deprecated.
Use filepicker instead, which is optimized for Ajax requests and displays as a dropdown list of files available (with thumbnail if images) to pick from.

PageMediaSelect was never intended to be used more than once, it was internally used for the Content area.

We already fixed this with the filepicker field (#750) and the completely rewrittenfile field (#748).

More details:

@maximelebreton
Copy link
Author

Already tried, and this is the same with filepicker

image


image
(the preview image prove that's a filepicker field)

@maximelebreton
Copy link
Author

But OK I'ill try the new file field! :)

@w00fz
Copy link
Member

w00fz commented Sep 19, 2016

Then that's not supposed to happen. It should only load data when you focus on the field. And also it wouldn't matter if you had 3 of those, the ajax request will be cached at that point. @flaviocopes ?

Btw, your first image of 0.5MB seems to be taking 6.5s for you to download (first screenshot). I'm not sure what connection you are trying with but one can download double that size on a regular 3G connection.
Are you sure your hosting provider is not capped?

@w00fz
Copy link
Member

w00fz commented Sep 20, 2016

And finally, we already started discussing internally how to best approach #779. We have a good plan of attack that would solve the upload of big images. They will be automatically resized and resampled the moment an image is selected for upload.
However, since this doesn't seem to be a common scenario happening and it will require some time for us to investigate and implement, it won't be something happening very soon.

@maximelebreton
Copy link
Author

maximelebreton commented Sep 20, 2016

Then that's not supposed to happen

in my case, it happens

since this doesn't seem to be a common scenario happening

uploading big images in a CMS is a very very common scenario, and performance is in the highest priorities for a good user experience. Customers no wonder if the back office is slow because of their upload size, and developpers expect from a CMS that is resilient.

You have maded an extremly powerful CMS, but please dont forget the basics ;)

edit: gosh sorry if I sound arrogant! (not my intention...)

@maximelebreton
Copy link
Author

Ok, the problem is more important than I thought.

If I have a slow connection, or a slow server, and the filepicker fields take 1 minute to load, the problem is "just" about speed.

BUT, if I edit my page (for example the title), without waiting that all the filepickers fields are loaded:
ALL of the filepickers data is LOSTED...

It's a HUGE design problem.

in this example, we can see:
1. the front end page is ok with all images
2. in the back end page, all the filepickers fields are filled
3. I refresh the back end page
4. I didn't wait for filepickers, and start to edit the title, and save
5. I refresh the front end page, and images are gone...

6. go back to the back end, and filepickers either aren't filled...

omg

@w00fz
Copy link
Member

w00fz commented Sep 21, 2016

Hey @maximelebreton,
we acknowledged the issue and like I said above we have plans for auto resizing down big images before they get uploaded.
I also see the issue of the file picker requesting to fill the dropdown for each one on load, which is why I left this issue open and labeled it.

There's nothing else we can do here until we get down to approach the issues, it might take time because there are other things we are looking at as well.

My only suggestion for you is to just go ahead and manually resize those images to a more reasonable size. Your site and slow connection will benefit from it immediately and it will at least keeps you running until we look into the issues.

Cheers.

@maximelebreton
Copy link
Author

I know you already acknowledge these two issues, but I raised a new one, who's when you're saving a page, if a filepicker not had time to load, his temporary 'blank value' is considered by Grav as a modification, and overwrite the initial value (who has not been modified)!

So this is a strong issue, but for me, this shows a bigger problem that is situated in the js core architecture.

Grav needs strong javascript foundations in admin for these kind of complex interactions, and I don't understand why most of Grav is built on top of best-class technologies like Twig, Markdown, Symfony, but the javascript admin client is not, when robust view library's like React would be so relevant?

What do you think?

@w00fz
Copy link
Member

w00fz commented Sep 21, 2016

There is no such thing as a blank temporary value. The value is always loaded separately in an hidden field with all the JSON data that gets stored, no matter if it takes 2 minutes for your images to load. So what you are describing has nothing to do with the JS architecture like you say, I think you are just wrongly interpreting what's going on. There is an issue with files and collections which I'm quite sure it's what you are experiencing (#775).

That said, I would love to use React of course, but It's a huge rework that we are not going to face at this moment. I'm also not quite sure it will solve your issues anyway.
React is just the view, you will still have to communicate with the server and given your slow connection you will still have to wait on responses.

Overall there is very little JS going on in the admin, aside from some simple DOM manipulation here and there and a few ajax requests, most of the work happens via PHP.

Just because we use vanilla JS for the most part, doesn't mean it's not built on top of best-class technologies or best practises. We use ES6 (including fetch), Mutations, Observers and Stateful-ness when needed.

React is an amazing piece of work and I would definitely love to use it at some point, but at the moment it doesn't feel like it would add much to Grav Admin. There is probably only a few places where React could help a lot: collections and arrays. Otherwise I think we are pretty good as is.
Besides, using React wouldn't magically solve all the problems.

So to recap, it seems like you are experiencing many different issues, some already reported by others, some related by your very slow connection (what connection do you have btw?). There is no such thing as values getting lost because the images take too long to load, in fact, the issue has nothing to do with JS at all, nor the architecture of it.

Please make your images smaller until this issues get resolved, that will solve many of your issues and at least we don't go in circle repeating the same things.

@maximelebreton
Copy link
Author

Sorry if i'm wrong, i'm just trying to help.
I greatly respect your work, and very enthusiastic about Grav.
Im more interested about make Grav better than quickly fixing my symptoms.
But if my feedbacks are not helpful, please tell me, I dont want to waste your time.


What i described as 'blank value' is the grav behaviour when there is no value (value is removed from the .md file)

and there is no duplication, like in #775

blueprint:

...
            header.projects:
              name: projects
              type: list

              fields:
                .artist:
                  type: text
                .track:
                  type: text
                .image:
                  type: filepicker
                  folder: '@self'
                .audio:
                  type: filepicker
                  folder: '@self'
                .category:
                  type: select
                .display:
                  type: toggle
...

original .md file:

projects:
    -
        artist: 'So Wax'
        track: 'U Who'
        image: 01_SoWax_1.jpg
        audio: 01_SoWax_UWho_SITEWEB_rt.mp3
        category: acoustique
        display: true
    -
        artist: Fables
        image: 06_Fables.jpg
        audio: 04_Fables_TheFall_SITEWEB.mp3
        category: amplifiee
        display: true

after .md file (if saving when filepicker fields aren't still loaded):

projects:
    -
        artist: 'So Wax'
        track: 'U Who'
        category: acoustique
        display: true
    -
        artist: Fables
        category: amplifiee
        display: true

@w00fz w00fz assigned w00fz and unassigned flaviocopes Oct 1, 2016
@w00fz w00fz closed this as completed in 74b756d Oct 1, 2016
@w00fz
Copy link
Member

w00fz commented Oct 1, 2016

I completely reworked the logic of this field. It was simply wrongly loading data via ajax at every page load. This was always meant to load data only when the field gained focus.
Now it's doing so.

It is also never going to show a preview image next to selected value of filepicker (on load). However if you activate the dropdown and get the data, then the selected field will get the preview.
This is so that slow connections don't waste any resources and when in the future we will handle proper thumbnails for admin, we can revisit this.

There is a dramatic boost in speed now, it is very optimized.

@fredrikekelund
Copy link
Contributor

So this is turning into a rather unwieldy issue, but I just wanted to ensure that the proposed solution you described for #779 wouldn't alter the images that ended up in the actual page folder, @w00fz?

Using very big images might not be optimal, but downsampling in the browser can produce very different results depending on your browser, and would just generally be unexpected behavior IMO

@flaviocopes
Copy link
Contributor

Thanks @w00fz for handling this, looks like a combination of not optimized filepicker + a slow response of the files list from the server.

Going to test it with big images + a slow connection simulation to make sure even with low performance and high load it's behaving as intended.

@maximelebreton
Copy link
Author

As a reminder, I never said I had a slow connection.
This slowness was mostly due to an old Planethoster's mutualized server with classic hard drives (not SSD), and no support of any cache service except filesystem.
Since I have migrated to a SSD server, it's about 10x faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants