[Mesa-dev] [PATCH v2 4/11] compiler/blob: Switch to init/finsih instead of create/destroy

Jason Ekstrand jason at jlekstrand.net
Wed Oct 11 20:52:17 UTC 2017


There's no reason why that tiny bit of memory needs to be on the heap.
We always put blob_reader on the stack, so why not do the same with the
writable blob.

v2 (Jason Ekstrand):
 - Fix the unit tests

---
 src/compiler/blob.c                      | 10 +---
 src/compiler/blob.h                      | 11 ++---
 src/compiler/glsl/shader_cache.cpp       | 39 +++++++--------
 src/compiler/glsl/tests/blob_test.c      | 82 ++++++++++++++++----------------
 src/mesa/state_tracker/st_shader_cache.c | 23 ++++-----
 5 files changed, 79 insertions(+), 86 deletions(-)

diff --git a/src/compiler/blob.c b/src/compiler/blob.c
index 041a352..0b42871 100644
--- a/src/compiler/blob.c
+++ b/src/compiler/blob.c
@@ -99,18 +99,12 @@ align_blob_reader(struct blob_reader *blob, size_t alignment)
    blob->current = blob->data + ALIGN(blob->current - blob->data, alignment);
 }
 
-struct blob *
-blob_create()
+void
+blob_init(struct blob *blob)
 {
-   struct blob *blob = (struct blob *) malloc(sizeof(struct blob));
-   if (blob == NULL)
-      return NULL;
-
    blob->data = NULL;
    blob->allocated = 0;
    blob->size = 0;
-
-   return blob;
 }
 
 bool
diff --git a/src/compiler/blob.h b/src/compiler/blob.h
index 4cbbb01..fd13a16 100644
--- a/src/compiler/blob.h
+++ b/src/compiler/blob.h
@@ -79,21 +79,18 @@ struct blob_reader {
 };
 
 /**
- * Create a new, empty blob.
- *
- * \return The new blob, (or NULL in case of allocation failure).
+ * Init a new, empty blob.
  */
-struct blob *
-blob_create(void);
+void
+blob_init(struct blob *blob);
 
 /**
  * Destroy a blob and free its memory.
  */
 static inline void
-blob_destroy(struct blob *blob)
+blob_finish(struct blob *blob)
 {
    free(blob->data);
-   free(blob);
 }
 
 /**
diff --git a/src/compiler/glsl/shader_cache.cpp b/src/compiler/glsl/shader_cache.cpp
index c9109aa..f3c7a57 100644
--- a/src/compiler/glsl/shader_cache.cpp
+++ b/src/compiler/glsl/shader_cache.cpp
@@ -1356,52 +1356,53 @@ shader_cache_write_program_metadata(struct gl_context *ctx,
    if (memcmp(prog->data->sha1, zero, sizeof(prog->data->sha1)) == 0)
       return;
 
-   struct blob *metadata = blob_create();
+   struct blob metadata;
+   blob_init(&metadata);
 
-   write_uniforms(metadata, prog);
+   write_uniforms(&metadata, prog);
 
-   write_hash_tables(metadata, prog);
+   write_hash_tables(&metadata, prog);
 
-   blob_write_uint32(metadata, prog->data->Version);
-   blob_write_uint32(metadata, prog->data->linked_stages);
+   blob_write_uint32(&metadata, prog->data->Version);
+   blob_write_uint32(&metadata, prog->data->linked_stages);
 
    for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
       struct gl_linked_shader *sh = prog->_LinkedShaders[i];
       if (sh) {
-         write_shader_metadata(metadata, sh);
+         write_shader_metadata(&metadata, sh);
 
          if (sh->Program->info.name)
-            blob_write_string(metadata, sh->Program->info.name);
+            blob_write_string(&metadata, sh->Program->info.name);
          else
-            blob_write_string(metadata, "");
+            blob_write_string(&metadata, "");
 
          if (sh->Program->info.label)
-            blob_write_string(metadata, sh->Program->info.label);
+            blob_write_string(&metadata, sh->Program->info.label);
          else
-            blob_write_string(metadata, "");
+            blob_write_string(&metadata, "");
 
          size_t s_info_size, s_info_ptrs;
          get_shader_info_and_pointer_sizes(&s_info_size, &s_info_ptrs,
                                            &sh->Program->info);
 
          /* Store shader info */
-         blob_write_bytes(metadata,
+         blob_write_bytes(&metadata,
                           ((char *) &sh->Program->info) + s_info_ptrs,
                           s_info_size - s_info_ptrs);
       }
    }
 
-   write_xfb(metadata, prog);
+   write_xfb(&metadata, prog);
 
-   write_uniform_remap_tables(metadata, prog);
+   write_uniform_remap_tables(&metadata, prog);
 
-   write_atomic_buffers(metadata, prog);
+   write_atomic_buffers(&metadata, prog);
 
-   write_buffer_blocks(metadata, prog);
+   write_buffer_blocks(&metadata, prog);
 
-   write_subroutines(metadata, prog);
+   write_subroutines(&metadata, prog);
 
-   write_program_resource_list(metadata, prog);
+   write_program_resource_list(&metadata, prog);
 
    struct cache_item_metadata cache_item_metadata;
    cache_item_metadata.type = CACHE_ITEM_TYPE_GLSL;
@@ -1423,7 +1424,7 @@ shader_cache_write_program_metadata(struct gl_context *ctx,
       }
    }
 
-   disk_cache_put(cache, prog->data->sha1, metadata->data, metadata->size,
+   disk_cache_put(cache, prog->data->sha1, metadata.data, metadata.size,
                   &cache_item_metadata);
 
    if (ctx->_Shader->Flags & GLSL_CACHE_INFO) {
@@ -1433,7 +1434,7 @@ shader_cache_write_program_metadata(struct gl_context *ctx,
 
 fail:
    free(cache_item_metadata.keys);
-   blob_destroy(metadata);
+   blob_finish(&metadata);
 }
 
 bool
diff --git a/src/compiler/glsl/tests/blob_test.c b/src/compiler/glsl/tests/blob_test.c
index df0e91a..d49f4a0 100644
--- a/src/compiler/glsl/tests/blob_test.c
+++ b/src/compiler/glsl/tests/blob_test.c
@@ -118,44 +118,44 @@ expect_equal_bytes(uint8_t *expected, uint8_t *actual,
 static void
 test_write_and_read_functions (void)
 {
-   struct blob *blob;
+   struct blob blob;
    struct blob_reader reader;
    uint8_t *reserved;
    size_t str_offset, uint_offset;
    uint8_t reserve_buf[sizeof(reserve_test_str)];
 
-   blob = blob_create();
+   blob_init(&blob);
 
    /*** Test blob by writing one of every possible kind of value. */
 
-   blob_write_bytes(blob, bytes_test_str, sizeof(bytes_test_str));
+   blob_write_bytes(&blob, bytes_test_str, sizeof(bytes_test_str));
 
-   reserved = blob_reserve_bytes(blob, sizeof(reserve_test_str));
+   reserved = blob_reserve_bytes(&blob, sizeof(reserve_test_str));
    memcpy(reserved, reserve_test_str, sizeof(reserve_test_str));
 
    /* Write a placeholder, (to be replaced later via overwrite_bytes) */
-   str_offset = blob->size;
-   blob_write_bytes(blob, placeholder_str, sizeof(placeholder_str));
+   str_offset = blob.size;
+   blob_write_bytes(&blob, placeholder_str, sizeof(placeholder_str));
 
-   blob_write_uint32(blob, uint32_test);
+   blob_write_uint32(&blob, uint32_test);
 
    /* Write a placeholder, (to be replaced later via overwrite_uint32) */
-   uint_offset = blob->size;
-   blob_write_uint32(blob, uint32_placeholder);
+   uint_offset = blob.size;
+   blob_write_uint32(&blob, uint32_placeholder);
 
-   blob_write_uint64(blob, uint64_test);
+   blob_write_uint64(&blob, uint64_test);
 
-   blob_write_intptr(blob, (intptr_t) blob);
+   blob_write_intptr(&blob, (intptr_t) &blob);
 
-   blob_write_string(blob, string_test_str);
+   blob_write_string(&blob, string_test_str);
 
    /* Finally, overwrite our placeholders. */
-   blob_overwrite_bytes(blob, str_offset, overwrite_test_str,
+   blob_overwrite_bytes(&blob, str_offset, overwrite_test_str,
                         sizeof(overwrite_test_str));
-   blob_overwrite_uint32(blob, uint_offset, uint32_overwrite);
+   blob_overwrite_uint32(&blob, uint_offset, uint32_overwrite);
 
    /*** Now read each value and verify. */
-   blob_reader_init(&reader, blob->data, blob->size);
+   blob_reader_init(&reader, blob.data, blob.size);
 
    expect_equal_str(bytes_test_str,
                     blob_read_bytes(&reader, sizeof(bytes_test_str)),
@@ -175,7 +175,7 @@ test_write_and_read_functions (void)
                 "blob_overwrite_uint32");
    expect_equal(uint64_test, blob_read_uint64(&reader),
                 "blob_write/read_uint64");
-   expect_equal((intptr_t) blob, blob_read_intptr(&reader),
+   expect_equal((intptr_t) &blob, blob_read_intptr(&reader),
                 "blob_write/read_intptr");
    expect_equal_str(string_test_str, blob_read_string(&reader),
                     "blob_write/read_string");
@@ -184,28 +184,28 @@ test_write_and_read_functions (void)
                 "read_consumes_all_bytes");
    expect_equal(false, reader.overrun, "read_does_not_overrun");
 
-   blob_destroy(blob);
+   blob_finish(&blob);
 }
 
 /* Test that data values are written and read with proper alignment. */
 static void
 test_alignment(void)
 {
-   struct blob *blob;
+   struct blob blob;
    struct blob_reader reader;
    uint8_t bytes[] = "ABCDEFGHIJKLMNOP";
    size_t delta, last, num_bytes;
 
-   blob = blob_create();
+   blob_init(&blob);
 
    /* First, write an intptr value to the blob and capture that size. This is
     * the expected offset between any pair of intptr values (if written with
     * alignment).
     */
-   blob_write_intptr(blob, (intptr_t) blob);
+   blob_write_intptr(&blob, (intptr_t) &blob);
 
-   delta = blob->size;
-   last = blob->size;
+   delta = blob.size;
+   last = blob.size;
 
    /* Then loop doing the following:
     *
@@ -215,56 +215,56 @@ test_alignment(void)
     *   2. Verify that that write results in an aligned size
     */
    for (num_bytes = 1; num_bytes < sizeof(intptr_t); num_bytes++) {
-      blob_write_bytes(blob, bytes, num_bytes);
+      blob_write_bytes(&blob, bytes, num_bytes);
 
-      expect_unequal(delta, blob->size - last, "unaligned write of bytes");
+      expect_unequal(delta, blob.size - last, "unaligned write of bytes");
 
-      blob_write_intptr(blob, (intptr_t) blob);
+      blob_write_intptr(&blob, (intptr_t) &blob);
 
-      expect_equal(2 * delta, blob->size - last, "aligned write of intptr");
+      expect_equal(2 * delta, blob.size - last, "aligned write of intptr");
 
-      last = blob->size;
+      last = blob.size;
    }
 
    /* Finally, test that reading also does proper alignment. Since we know
     * that values were written with all the right alignment, all we have to do
     * here is verify that correct values are read.
     */
-   blob_reader_init(&reader, blob->data, blob->size);
+   blob_reader_init(&reader, blob.data, blob.size);
 
-   expect_equal((intptr_t) blob, blob_read_intptr(&reader),
+   expect_equal((intptr_t) &blob, blob_read_intptr(&reader),
                 "read of initial, aligned intptr_t");
 
    for (num_bytes = 1; num_bytes < sizeof(intptr_t); num_bytes++) {
       expect_equal_bytes(bytes, blob_read_bytes(&reader, num_bytes),
                          num_bytes, "unaligned read of bytes");
-      expect_equal((intptr_t) blob, blob_read_intptr(&reader),
+      expect_equal((intptr_t) &blob, blob_read_intptr(&reader),
                    "aligned read of intptr_t");
    }
 
-   blob_destroy(blob);
+   blob_finish(&blob);
 }
 
 /* Test that we detect overrun. */
 static void
 test_overrun(void)
 {
-   struct blob *blob;
+   struct blob blob;
    struct blob_reader reader;
    uint32_t value = 0xdeadbeef;
 
-   blob = blob_create();
+   blob_init(&blob);
 
-   blob_write_uint32(blob, value);
+   blob_write_uint32(&blob, value);
 
-   blob_reader_init(&reader, blob->data, blob->size);
+   blob_reader_init(&reader, blob.data, blob.size);
 
    expect_equal(value, blob_read_uint32(&reader), "read before overrun");
    expect_equal(false, reader.overrun, "overrun flag not set");
    expect_equal(0, blob_read_uint32(&reader), "read at overrun");
    expect_equal(true, reader.overrun, "overrun flag set");
 
-   blob_destroy(blob);
+   blob_finish(&blob);
 }
 
 /* Test that we can read and write some large objects, (exercising the code in
@@ -274,14 +274,14 @@ static void
 test_big_objects(void)
 {
    void *ctx = ralloc_context(NULL);
-   struct blob *blob;
+   struct blob blob;
    struct blob_reader reader;
    int size = 1000;
    int count = 1000;
    size_t i;
    char *buf;
 
-   blob = blob_create();
+   blob_init(&blob);
 
    /* Initialize our buffer. */
    buf = ralloc_size(ctx, size);
@@ -291,10 +291,10 @@ test_big_objects(void)
 
    /* Write it many times. */
    for (i = 0; i < count; i++) {
-      blob_write_bytes(blob, buf, size);
+      blob_write_bytes(&blob, buf, size);
    }
 
-   blob_reader_init(&reader, blob->data, blob->size);
+   blob_reader_init(&reader, blob.data, blob.size);
 
    /* Read and verify it many times. */
    for (i = 0; i < count; i++) {
@@ -308,7 +308,7 @@ test_big_objects(void)
    expect_equal(false, reader.overrun,
                 "overrun flag not set reading large objects");
 
-   blob_destroy(blob);
+   blob_finish(&blob);
    ralloc_free(ctx);
 }
 
diff --git a/src/mesa/state_tracker/st_shader_cache.c b/src/mesa/state_tracker/st_shader_cache.c
index 2c38f27..2e9b7cc 100644
--- a/src/mesa/state_tracker/st_shader_cache.c
+++ b/src/mesa/state_tracker/st_shader_cache.c
@@ -69,21 +69,22 @@ st_store_tgsi_in_disk_cache(struct st_context *st, struct gl_program *prog,
       return;
 
    unsigned char *sha1;
-   struct blob *blob = blob_create();
+   struct blob blob;
+   blob_init(&blob);
 
    switch (prog->info.stage) {
    case MESA_SHADER_VERTEX: {
       struct st_vertex_program *stvp = (struct st_vertex_program *) prog;
       sha1 = stvp->sha1;
 
-      blob_write_uint32(blob, stvp->num_inputs);
-      blob_write_bytes(blob, stvp->index_to_input,
+      blob_write_uint32(&blob, stvp->num_inputs);
+      blob_write_bytes(&blob, stvp->index_to_input,
                        sizeof(stvp->index_to_input));
-      blob_write_bytes(blob, stvp->result_to_output,
+      blob_write_bytes(&blob, stvp->result_to_output,
                        sizeof(stvp->result_to_output));
 
-      write_stream_out_to_cache(blob, &stvp->tgsi);
-      write_tgsi_to_cache(blob, &stvp->tgsi, st, sha1, num_tokens);
+      write_stream_out_to_cache(&blob, &stvp->tgsi);
+      write_tgsi_to_cache(&blob, &stvp->tgsi, st, sha1, num_tokens);
       break;
    }
    case MESA_SHADER_TESS_CTRL:
@@ -92,22 +93,22 @@ st_store_tgsi_in_disk_cache(struct st_context *st, struct gl_program *prog,
       struct st_common_program *p = st_common_program(prog);
       sha1 = p->sha1;
 
-      write_stream_out_to_cache(blob, out_state);
-      write_tgsi_to_cache(blob, out_state, st, sha1, num_tokens);
+      write_stream_out_to_cache(&blob, out_state);
+      write_tgsi_to_cache(&blob, out_state, st, sha1, num_tokens);
       break;
    }
    case MESA_SHADER_FRAGMENT: {
       struct st_fragment_program *stfp = (struct st_fragment_program *) prog;
       sha1 = stfp->sha1;
 
-      write_tgsi_to_cache(blob, &stfp->tgsi, st, sha1, num_tokens);
+      write_tgsi_to_cache(&blob, &stfp->tgsi, st, sha1, num_tokens);
       break;
    }
    case MESA_SHADER_COMPUTE: {
       struct st_compute_program *stcp = (struct st_compute_program *) prog;
       sha1 = stcp->sha1;
 
-      write_tgsi_to_cache(blob, out_state, st, sha1, num_tokens);
+      write_tgsi_to_cache(&blob, out_state, st, sha1, num_tokens);
       break;
    }
    default:
@@ -121,7 +122,7 @@ st_store_tgsi_in_disk_cache(struct st_context *st, struct gl_program *prog,
               _mesa_shader_stage_to_string(prog->info.stage), sha1_buf);
    }
 
-   blob_destroy(blob);
+   blob_finish(&blob);
 }
 
 static void
-- 
2.5.0.400.gff86faf



More information about the mesa-dev mailing list