Skip to content

Conversation

@Glandos
Copy link
Contributor

@Glandos Glandos commented Nov 6, 2022

Filesystem::isValidPath seems to be useless here, since we already have our file info anyway.

After applying #34910 it still gives me the following improvement, when running preview generator on a folder with 1473 images, all already generated.

  • Before:
 Performance counter stats for './occ preview:generate-all -vv --path=/users/files/photos/2008 - 09 - Viet Nam/' (5 runs):

         13 124,93 msec task-clock                       #    0,653 CPUs utilized            ( +-  1,17% )
             5 826      context-switches                 #  451,999 /sec                     ( +-  1,01% )
               719      cpu-migrations                   #   55,782 /sec                     ( +-  6,37% )
            20 846      page-faults                      #    1,617 K/sec                    ( +-  0,01% )
    24 144 069 088      cycles                           #    1,873 GHz                      ( +-  1,17% )  (50,38%)
     5 539 208 540      instructions                     #    0,23  insn per cycle           ( +-  0,17% )  (74,88%)
     1 124 152 538      branches                         #   87,215 M/sec                    ( +-  0,16% )  (74,30%)
       164 879 701      branch-misses                    #   14,65% of all branches          ( +-  0,31% )  (75,34%)

            20,084 +- 0,267 seconds time elapsed  ( +-  1,33% )
  • After:
 Performance counter stats for './occ preview:generate-all -vv --path=/users/files/photos/2008 - 09 - Viet Nam/' (5 runs):

         10 918,60 msec task-clock                       #    0,615 CPUs utilized            ( +-  1,20% )
             5 702      context-switches                 #  531,419 /sec                     ( +-  1,06% )
               802      cpu-migrations                   #   74,745 /sec                     ( +-  8,95% )
            20 827      page-faults                      #    1,941 K/sec                    ( +-  0,01% )
    20 074 183 445      cycles                           #    1,871 GHz                      ( +-  1,19% )  (49,12%)
     4 660 549 008      instructions                     #    0,24  insn per cycle           ( +-  0,17% )  (74,39%)
       928 891 708      branches                         #   86,571 M/sec                    ( +-  0,13% )  (75,05%)
       137 892 289      branch-misses                    #   14,85% of all branches          ( +-  0,40% )  (75,87%)

            17,768 +- 0,242 seconds time elapsed  ( +-  1,36% )

It's a small use case, but 10% improvement seems valuable to me :)

@szaimen szaimen added this to the Nextcloud 26 milestone Nov 7, 2022
@szaimen szaimen added the 3. to review Waiting for reviews label Nov 7, 2022
@szaimen szaimen requested review from a team, CarlSchwan, blizzz and icewind1991 and removed request for a team November 7, 2022 00:05
@Glandos
Copy link
Contributor Author

Glandos commented Nov 7, 2022

Is there anything I can do for the failing tests? As far as I understand:

  • Performance testing / performance-8.0 (pull_request) => can't find remote ref
  • S3 primary storage / php8.0-objectstore_multibucket-minio (pull_request) => Failed to connect to localhost port 9000
  • S3 primary storage / s3-primary-summary (pull_request) => if true; then exit 1; fi, so designed to fail
  • continuous-integration/drone/pr => failed on acceptance-app-files-sharing but this seems to fail at random

@icewind1991
Copy link
Member

I've restarted the failing S3 test, you can ignore the other 2 failures

@szaimen szaimen merged commit 9d45845 into nextcloud:master Nov 11, 2022
@welcome
Copy link

welcome bot commented Nov 11, 2022

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants