On Sat, 2017-03-11 at 17:08 +0000, Emil Velikov wrote:
> On 10 March 2017 at 10:51, Grazvydas Ignotas <nota...@gmail.com>
> wrote:
> >
> > On Fri, Mar 10, 2017 at 6:23 AM, Timothy Arceri <tarceri@itsqueeze.
> > com> wrote:
> > >
> > > On 07/03/17 12:25, Alan Swanson wrote:
> > > >
> > > >
> > > > Still using random selection of two-character subdirectory in
> > > > which
> > > > to check cache files rather than scanning entire cache.
> > > >
> > > > v2: Factor out double strlen call
> > > > ---
> > > > src/util/disk_cache.c | 78
> > > > +++++++++++++++++++++++----------------------------
> > > > 1 file changed, 35 insertions(+), 43 deletions(-)
> > > >
> > > > diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
> > > > index 31a9336582..2a923be3dc 100644
> > > > --- a/src/util/disk_cache.c
> > > > +++ b/src/util/disk_cache.c
[SNIP]
> > > > char *filename;
> > > >
> > > > dir = opendir(dir_path);
> > > > if (dir == NULL)
> > > > return NULL;
> > > >
> > > > - count = 0;
> > > > -
> > > > - while (1) {
> > > > - entry = readdir(dir);
> > > > - if (entry == NULL)
> > > > - break;
> > > > - if (!predicate(entry, dir_path))
> > > > - continue;
> > > > -
> > > > - count++;
> > > > - }
> > > > -
> > > > - if (count == 0) {
> > > > - closedir(dir);
> > > > - return NULL;
> > > > - }
> > > > -
> > > > - victim = rand() % count;
> > > > -
> > > > - rewinddir(dir);
> > > > - count = 0;
> > > > -
> > > > while (1) {
> > > > entry = readdir(dir);
> > > > if (entry == NULL)
> > > > break;
> > > > if (!predicate(entry, dir_path))
> > > > continue;
> > > > - if (count == victim)
> > > > - break;
> > > >
> > > > - count++;
> > > > + if (fstatat(dirfd(dir), entry->d_name, &sb, 0) == 0) {
> > > > + if (!lru_atime || (sb.st_atime < lru_atime)) {
> > > > + len = strlen(entry->d_name) + 1;
> > > > + tmp = realloc(lru_name, len);
> > > > + if (tmp) {
> > > > + lru_name = tmp;
> > >
> > >
> > > I don't think we need tmp here. Why not use lru_name directly?
> >
> > It *is* needed, otherwise if realloc fails, you'll leak old memory.
> >
> Indeed. Alternatively one can avoid all the strlen/realloc/free
> dances
> by using NAME_MAX.
>
> As per the POSIX manual
> "The array d_name is of unspecified size, but shall contain a
> filename
> of at most {NAME_MAX} bytes followed by a terminating null byte."
>
> Not 100% sure if it'll play well with Hurd and similar platforms :-\
But here it's a very simple dance. The realloc should mostly become a
noop, at least with glibc, after the first entry in cache and just
activating for any .tmp files. Using MAX always seems like a lazy over
allocation to me.
A follow on patch will pass the sb and len to predicate functions
instead saving them from calling fstat and strlen on same entry again.
Will post that this weekend when I get back home.
--
Alan.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
