[Mesa-dev] [PATCH 2/2] anv: Use build-id for pipeline cache UUID.

[Chad Versace]

On Tue 14 Feb 2017, Kenneth Graunke wrote:
> On Tuesday, February 14, 2017 12:38:45 PM PST Chad Versace wrote:
> > On Tue 14 Feb 2017, Matt Turner wrote:
> > 
> > 
> > >  static bool
> > > -anv_get_function_timestamp(void *ptr, uint32_t* timestamp)
> > > +anv_device_get_cache_uuid(void *uuid)
> > >  {
> > > -   Dl_info info;
> > > -   struct stat st;
> > > -   if (!dladdr(ptr, &info) || !info.dli_fname)
> > > +   const struct note *note = build_id_find_nhdr("libvulkan_intel.so");
> > > +   if (!note)
> > >        return false;
> > >  
> > > -   if (stat(info.dli_fname, &st))
> > > +   unsigned len = build_id_length(note);
> > > +   if (len < VK_UUID_SIZE)
> > >        return false;
> > >  
> > > -   *timestamp = st.st_mtim.tv_sec;
> > > -   return true;
> > > -}
> > > -
> > > -static bool
> > > -anv_device_get_cache_uuid(void *uuid)
> > > -{
> > > -   uint32_t timestamp;
> > > -
> > > -   memset(uuid, 0, VK_UUID_SIZE);
> > > -   if (!anv_get_function_timestamp(anv_device_get_cache_uuid, &timestamp))
> > > +   unsigned char *build_id = malloc(len);
> > > +   if (!build_id)
> > >        return false;
> > >  
> > > -   snprintf(uuid, VK_UUID_SIZE, "anv-%d", timestamp);
> > > +   build_id_read(note, build_id);
> > > +
> > > +   memcpy(uuid, build_id, VK_UUID_SIZE);
> > > +   free(build_id);
> > 
> > The Vulkan spec frowns on memory allocations when not needed. If you
> > must allocate memory here, then it should be through the VkInstance
> > allocation callbacks. However, it's best to avoid the allocation by
> > adding a size_t parameter, à la snprintf, to build_id_read().
> > 
> > Otherwise, the patch looks good to me.
> 
> You're worried about the performance of anv_physical_device_init()?
> This doesn't happen on lookup...just driver start up...

I'm not worried about performance here. That would be foolish.

I'm concerned about complying the expectations provided to advanced
users by the Vulkan spec. Anvil has a good record of meeting those
expectations so far. There are exactly 4 calls to malloc in
src/intel/vulkan and, arguably, there should be only 2.

    $ git grep malloc src/intel/vulkan
    
    # correct and needed
    anv_device.c:   return malloc(size);

    # ok. this is debug code
    anv_dump.c:   uint8_t *row = malloc(image->extent.width * 3);

    # we should fix these
    anv_pipeline.c:      spec_entries = malloc(num_spec_entries * sizeof(*spec_entries));
    anv_pipeline.c:         malloc(prog_data->nr_params * sizeof(union gl_constant_value *));