Refactor away more array_shift for performance

[SubJunk]

Some of the uses of array_shift have now been replaced with more performant syntax, but there are some that aren't so easy to do. The next methods (next_image, next_album, next_comment, maybe more), all rely on array_shift on global variables, and then they maintain some context of the loop using statements and temporary variables that get restored at the end of the loop.

I think it would be good to refactor these too, since it will be impacting performance when those collections get large, and the code pattern isn't the most elegant IMO. I can try to do it but since it would take more time I wanted to check if you guys support that change?

[sbillard]

Is the performance issue just that the array gets reindexed if the indices are numeric? If so, then the best bet would be to make the indices of those arrays into strings. (I have not looked at that yet.)

[SubJunk]

@sbillard I'm not sure what the exact reason is, but I know that this change https://github.com/zenphoto/zenphoto/commit/5feba86224eb8ba03391d3ffe41410b8c08d01ac was very big for my server, it made it go from CPU spiking so hard that it would timeout requests, to sitting nicely below 30% CPU all the time.

Edit: Some of the performance benefit in that change would be due to the break too, depending on how lucky the user got in the order of the array

[acrylian]

@Subjunk: I support everything that improves performances. I read about the performance issue with array_shift and numeric indexed arrays. But never noticed any issue this as surely non of my ZP sites are as large as yours and ZP "feels" generally fast. About what size we are talking actually? Just to have a number.

[sbillard]

I made changes to the next_object() functions. These did not seem to make much difference in performance. The variance in performance was almost as big as the mean savings. This is probably because the arrays these functions are working on are not very large--only the size of the number of objects on a page.

[SubJunk]

My biggest ZP site is https://dualmonitorbackgrounds.com/ which has 16878 users, each of them have an album. There are 6135 images and 2842 tags

Edit: One of my slowest loading pages is zp-core/admin-edit.php, I'm not sure why yet. I'm also in the process of synchronising my code with the latest zp releases since I'm quite out of date

Edit 2: In the last week we had 4.89m requests with 240GB data served

[acrylian]

@Subjunk Yes, I know that site but wasn't aware somehow that it is yours. I think that's the largest ZP site I know.

One of my slowest loading pages is zp-core/admin-edit.php, I'm not sure why yet. I'm also in the process of synchronising my code with the latest zp releases since I'm quite out of date

Yes, 1.4.5.x is a bit behind ;-) There are several "pages" on admin-edit.php. Also the main albums page does a database filesystem match which might be a bit heavier on your large site. Be sure to disable albums thumbs and/or random display of album thumbs. But there have some other changes regarding the move/copy and the album selector.

[SubJunk]

That's cool to know :) I've had trouble getting past 1.4.5.x in the past because of customizations I did, but that was when I was using SVN so I might be more successful with Git. I'm making a branch on my ZP fork that contains my customizations, and I'll attempt to merge newer releases into that one by one.

I manually disabled the database filesystem match a long time ago (I think @sbillard helped with that, along with some other performance-related customizations)

[acrylian]

@subjunk Btw, could you please send me a mail directly?