Skip to content
Snippets Groups Projects

vkd3d-shader/spirv: Do not reallocate the SPIR-V program.

Merged Giovanni Mascellani requested to merge giomasce/vkd3d:vesuvio into master
1 unresolved thread

I ran the compilation of ~1000 DXBC-TPF shaders randomly taken from my collection and measured the performance using callgrind and the kcachegrind "cycle count" estimation.

BEFORE:

  • 1,846,641,596 cycles
  • 1,845,635,336 cycles
  • 1,841,335,225 cycles

AFTER:

  • 1,764,035,136 cycles
  • 1,767,948,767 cycles
  • 1,773,927,734 cycles

So callgrind would esitemate a 3.6% improvement at least.

The counterpoint is that the caller might get an allocation that is potentially bigger than necessary. I would expect that allocation to be rather short-lived anyway, so that's probably not a problem.

Merge request reports

Merge request pipeline #32097 skipped

Merge request pipeline skipped for 9777c8bc

Approved by

Merged by Henri VerbeetHenri Verbeet 5 months ago (Sep 20, 2024 3:31pm UTC)

Merge details

  • Changes merged into master with 9777c8bc.
  • Did not delete the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Giovanni Mascellani approved this merge request

    approved this merge request

    • @@ -2086,19 +2085,11 @@ static bool vkd3d_spirv_compile_module(struct vkd3d_spirv_builder *builder,
               return false;
           }
       
      -    if (!(code = vkd3d_calloc(stream.word_count, sizeof(*code))))
      -    {
      -        vkd3d_spirv_stream_free(&stream);
      -        return false;
      -    }
      -
      -    size = stream.word_count * sizeof(*code);
      -    memcpy(code, stream.words, size);
      +    spirv->code = stream.words;
      +    spirv->size = stream.word_count * sizeof(*code);
      +    stream.words = NULL;
           vkd3d_spirv_stream_free(&stream);
       
      -    spirv->code = code;
      -    spirv->size = size;
      -
           return true;
       }
       

      Should we introduce a helper function for this? E.g. vkd3d_shader_code_from_spirv_stream(), along the lines of vkd3d_shader_code_from_string_buffer().

      There's perhaps also an argument that vkd3d_spirv_stream should be built on top of vkd3d_bytecode_buffer, but that would be a somewhat larger change.

    • I don't feel a strong need for a helper, given that AFAIK it would be used only here. It makes more sense for vkd3d_shader_code_from_string_buffer() because struct vkd3d_string_buffer and struct vkd3d_shader_code are meant to be reusable components. I don't think there are plans to make struct vkd3d_spirv_stream reusable across vkd3d-shader.

      However, if you think that would be better I can certainly do that.

      There's perhaps also an argument that vkd3d_spirv_stream should be built on top of vkd3d_bytecode_buffer, but that would be a somewhat larger change.

      Yeah, I would consider that a bit out of scope here. It might be taken into consideration for the future.

    • Please register or sign in to reply
  • added 1 commit

    • 64022997 - vkd3d-shader/spirv: Do not reallocate the SPIR-V program.

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • Henri Verbeet added 12 commits

    added 12 commits

    • 64022997...637a3cab - 10 commits from branch wine:master
    • 35d3161f - vkd3d-shader/spirv: Propagate errors from vkd3d_spirv_stream_append().
    • 9777c8bc - vkd3d-shader/spirv: Do not reallocate the SPIR-V program.

    Compare with previous version

Please register or sign in to reply
Loading