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

[Emil Velikov]

On 14 February 2017 at 21:02, Chad Versace <chadversary at chromium.org> wrote:
> 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.
Hi Chad, pardon for dropping in uninvited:

Spec mandates that only the implementation should use the allocation
callbacks, and not any of it's dependencies (say something in the C
runtime/other library), correct ?
Or does it implicitly that "Thou Shalt override the weak
[mc]alloc/friends symbols provided by your C runtime" ?

Although the src/intel is [almost] perfect, the i965 compiler still
explicitly and implicitly uses [mc]alloc. Not mean to be bashful, just
pointing out what may not be obvious at first.

Thanks
Emil