[Mesa-dev] util/disk_cache: pass predicate functions file stats directly (v3)

Submitted by Alan Swanson on March 16, 2017, 5:36 p.m.

Details

Message ID 20170316173600.22071-1-reiver@improbability.net
State New
Headers show
Series "Series without cover letter" ( rev: 6 ) in Mesa

Not browsing as part of any series.

Commit Message

Alan Swanson March 16, 2017, 5:36 p.m.
Since switching to LRU eviction the only user of these predicate
functions now resolves directory entry stats itself so pass them
directly saving calling fstat and strlen twice (and the
expensive strlen is skipped entirely if access time is newer).

v2: Update for empty cache dir detection changes
v3: Fix passing string length to predicate with the +1 for NULL
    termination and also pass sb as pointer
---
That +1 was embarrassingly careless. Thanks for catching.

 src/util/disk_cache.c | 55 ++++++++++++++++++++-------------------------------
 1 file changed, 21 insertions(+), 34 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index e015e56f5e..c2ed58047a 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -481,8 +481,9 @@  make_cache_file_directory(struct disk_cache *cache, const cache_key key)
  */
 static char *
 choose_lru_file_matching(const char *dir_path,
-                         bool (*predicate)(const struct dirent *,
-                                           const char *dir_path))
+                         bool (*predicate)(const char *dir_path,
+                                           const struct stat *,
+                                           const char *, const size_t))
 {
    DIR *dir;
    struct dirent *entry;
@@ -498,17 +499,19 @@  choose_lru_file_matching(const char *dir_path,
       entry = readdir(dir);
       if (entry == NULL)
          break;
-      if (!predicate(entry, dir_path))
-         continue;
 
       struct stat sb;
       if (fstatat(dirfd(dir), entry->d_name, &sb, 0) == 0) {
          if (!lru_atime || (sb.st_atime < lru_atime)) {
-            size_t len = strlen(entry->d_name) + 1;
-            char *tmp = realloc(lru_name, len);
+            size_t len = strlen(entry->d_name);
+
+            if (!predicate(dir_path, sb, entry->d_name, len))
+               continue;
+
+            char *tmp = realloc(lru_name, len + 1);
             if (tmp) {
                lru_name = tmp;
-               memcpy(lru_name, entry->d_name, len);
+               memcpy(lru_name, entry->d_name, len + 1);
                lru_atime = sb.st_atime;
             }
          }
@@ -533,21 +536,13 @@  choose_lru_file_matching(const char *dir_path,
  * ".tmp"
  */
 static bool
-is_regular_non_tmp_file(const struct dirent *entry, const char *path)
+is_regular_non_tmp_file(const char *path, const struct stat *sb,
+                        const char *d_name, const size_t len)
 {
-   char *filename;
-   if (asprintf(&filename, "%s/%s", path, entry->d_name) == -1)
-      return false;
-
-   struct stat sb;
-   int res = stat(filename, &sb);
-   free(filename);
-
-   if (res == -1 || !S_ISREG(sb.st_mode))
+   if (!S_ISREG(sb->st_mode))
       return false;
 
-   size_t len = strlen (entry->d_name);
-   if (len >= 4 && strcmp(&entry->d_name[len-4], ".tmp") == 0)
+   if (len >= 4 && strcmp(&d_name[len-4], ".tmp") == 0)
       return false;
 
    return true;
@@ -579,29 +574,21 @@  unlink_lru_file_from_directory(const char *path)
  * special name of ".."). We also return false if the dir is empty.
  */
 static bool
-is_two_character_sub_directory(const struct dirent *entry, const char *path)
+is_two_character_sub_directory(const char *path, const struct stat *sb,
+                               const char *d_name, const size_t len)
 {
-   char *subdir;
-   if (asprintf(&subdir, "%s/%s", path, entry->d_name) == -1)
+   if (!S_ISDIR(sb->st_mode))
       return false;
 
-   struct stat sb;
-   int res = stat(subdir, &sb);
-   if (res == -1 || !S_ISDIR(sb.st_mode)) {
-      free(subdir);
+   if (len != 2)
       return false;
-   }
 
-   if (strlen(entry->d_name) != 2) {
-      free(subdir);
+   if (strcmp(d_name, "..") == 0)
       return false;
-   }
 
-   if (strcmp(entry->d_name, "..") == 0) {
-      free(subdir);
+   char *subdir;
+   if (asprintf(&subdir, "%s/%s", path, d_name) == -1)
       return false;
-   }
-
    DIR *dir = opendir(subdir);
    free(subdir);
 

Comments

On Thu, Mar 16, 2017 at 7:36 PM, Alan Swanson <reiver@improbability.net> wrote:
> Since switching to LRU eviction the only user of these predicate
> functions now resolves directory entry stats itself so pass them
> directly saving calling fstat and strlen twice (and the
> expensive strlen is skipped entirely if access time is newer).
>
> v2: Update for empty cache dir detection changes
> v3: Fix passing string length to predicate with the +1 for NULL
>     termination and also pass sb as pointer
> ---
> That +1 was embarrassingly careless. Thanks for catching.

Now it doesn't seem to compile, sent some wrong version?

GraÅžvydas

>
>  src/util/disk_cache.c | 55 ++++++++++++++++++++-------------------------------
>  1 file changed, 21 insertions(+), 34 deletions(-)
>
> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
> index e015e56f5e..c2ed58047a 100644
> --- a/src/util/disk_cache.c
> +++ b/src/util/disk_cache.c
> @@ -481,8 +481,9 @@ make_cache_file_directory(struct disk_cache *cache, const cache_key key)
>   */
>  static char *
>  choose_lru_file_matching(const char *dir_path,
> -                         bool (*predicate)(const struct dirent *,
> -                                           const char *dir_path))
> +                         bool (*predicate)(const char *dir_path,
> +                                           const struct stat *,
> +                                           const char *, const size_t))
>  {
>     DIR *dir;
>     struct dirent *entry;
> @@ -498,17 +499,19 @@ choose_lru_file_matching(const char *dir_path,
>        entry = readdir(dir);
>        if (entry == NULL)
>           break;
> -      if (!predicate(entry, dir_path))
> -         continue;
>
>        struct stat sb;
>        if (fstatat(dirfd(dir), entry->d_name, &sb, 0) == 0) {
>           if (!lru_atime || (sb.st_atime < lru_atime)) {
> -            size_t len = strlen(entry->d_name) + 1;
> -            char *tmp = realloc(lru_name, len);
> +            size_t len = strlen(entry->d_name);
> +
> +            if (!predicate(dir_path, sb, entry->d_name, len))
> +               continue;
> +
> +            char *tmp = realloc(lru_name, len + 1);
>              if (tmp) {
>                 lru_name = tmp;
> -               memcpy(lru_name, entry->d_name, len);
> +               memcpy(lru_name, entry->d_name, len + 1);
>                 lru_atime = sb.st_atime;
>              }
>           }
> @@ -533,21 +536,13 @@ choose_lru_file_matching(const char *dir_path,
>   * ".tmp"
>   */
>  static bool
> -is_regular_non_tmp_file(const struct dirent *entry, const char *path)
> +is_regular_non_tmp_file(const char *path, const struct stat *sb,
> +                        const char *d_name, const size_t len)
>  {
> -   char *filename;
> -   if (asprintf(&filename, "%s/%s", path, entry->d_name) == -1)
> -      return false;
> -
> -   struct stat sb;
> -   int res = stat(filename, &sb);
> -   free(filename);
> -
> -   if (res == -1 || !S_ISREG(sb.st_mode))
> +   if (!S_ISREG(sb->st_mode))
>        return false;
>
> -   size_t len = strlen (entry->d_name);
> -   if (len >= 4 && strcmp(&entry->d_name[len-4], ".tmp") == 0)
> +   if (len >= 4 && strcmp(&d_name[len-4], ".tmp") == 0)
>        return false;
>
>     return true;
> @@ -579,29 +574,21 @@ unlink_lru_file_from_directory(const char *path)
>   * special name of ".."). We also return false if the dir is empty.
>   */
>  static bool
> -is_two_character_sub_directory(const struct dirent *entry, const char *path)
> +is_two_character_sub_directory(const char *path, const struct stat *sb,
> +                               const char *d_name, const size_t len)
>  {
> -   char *subdir;
> -   if (asprintf(&subdir, "%s/%s", path, entry->d_name) == -1)
> +   if (!S_ISDIR(sb->st_mode))
>        return false;
>
> -   struct stat sb;
> -   int res = stat(subdir, &sb);
> -   if (res == -1 || !S_ISDIR(sb.st_mode)) {
> -      free(subdir);
> +   if (len != 2)
>        return false;
> -   }
>
> -   if (strlen(entry->d_name) != 2) {
> -      free(subdir);
> +   if (strcmp(d_name, "..") == 0)
>        return false;
> -   }
>
> -   if (strcmp(entry->d_name, "..") == 0) {
> -      free(subdir);
> +   char *subdir;
> +   if (asprintf(&subdir, "%s/%s", path, d_name) == -1)
>        return false;
> -   }
> -
>     DIR *dir = opendir(subdir);
>     free(subdir);
>
> --
> 2.11.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev