<div dir="ltr">Whoops, I discovered another issue:<br><div><div class="gmail_extra"><br><br><div class="gmail_quote">On 29 January 2014 01:24, Tapani Pälli <span dir="ltr"><<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Class will be used by the shader binary cache implementation.<br>
<br>
Signed-off-by: Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>><br>
---<br>
src/glsl/memory_writer.h | 188 +++++++++++++++++++++++++++++++++++++++++++++++<br>
1 file changed, 188 insertions(+)<br>
create mode 100644 src/glsl/memory_writer.h<br>
<br>
diff --git a/src/glsl/memory_writer.h b/src/glsl/memory_writer.h<br>
new file mode 100644<br>
index 0000000..979169f<br>
--- /dev/null<br>
+++ b/src/glsl/memory_writer.h<br>
@@ -0,0 +1,188 @@<br>
+/* -*- c++ -*- */<br>
+/*<br>
+ * Copyright © 2013 Intel Corporation<br>
+ *<br>
+ * Permission is hereby granted, free of charge, to any person obtaining a<br>
+ * copy of this software and associated documentation files (the "Software"),<br>
+ * to deal in the Software without restriction, including without limitation<br>
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,<br>
+ * and/or sell copies of the Software, and to permit persons to whom the<br>
+ * Software is furnished to do so, subject to the following conditions:<br>
+ *<br>
+ * The above copyright notice and this permission notice (including the next<br>
+ * paragraph) shall be included in all copies or substantial portions of the<br>
+ * Software.<br>
+ *<br>
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR<br>
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,<br>
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL<br>
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER<br>
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING<br>
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER<br>
+ * DEALINGS IN THE SOFTWARE.<br>
+ */<br>
+<br>
+#pragma once<br>
+#ifndef MEMORY_WRITER_H<br>
+#define MEMORY_WRITER_H<br>
+<br>
+#include <stdlib.h><br>
+#include <unistd.h><br>
+#include <string.h><br>
+<br>
+#include "main/hash_table.h"<br>
+<br>
+#ifdef __cplusplus<br>
+/**<br>
+ * Helper class for writing data to memory<br>
+ *<br>
+ * This class maintains a dynamically-sized memory buffer and allows<br>
+ * for data to be efficiently appended to it with automatic resizing.<br>
+ */<br>
+class memory_writer<br>
+{<br>
+public:<br>
+ memory_writer() :<br>
+ memory(NULL),<br>
+ curr_size(0),<br>
+ pos(0)<br>
+ {<br>
+ data_hash = _mesa_hash_table_create(0, int_equal);<br>
+ hash_value = _mesa_hash_data(this, sizeof(memory_writer));<br>
+ }<br>
+<br>
+ ~memory_writer()<br>
+ {<br>
+ free(memory);<br>
+ _mesa_hash_table_destroy(data_hash, NULL);<br>
+ }<br>
+<br>
+ /* user wants to claim the memory */<br>
+ char *release_memory(size_t *size)<br>
+ {<br>
+ /* final realloc to free allocated but unused memory */<br>
+ char *result = (char *) realloc(memory, pos);<br>
+ *size = pos;<br>
+ memory = NULL;<br>
+ curr_size = 0;<br>
+ pos = 0;<br>
+ return result;<br>
+ }<br>
+<br>
+/**<br>
+ * write functions per type<br>
+ */<br>
+#define DECL_WRITER(type) void write_ ##type (const type data) {\<br>
+ write(&data, sizeof(type));\<br>
+}<br>
+<br>
+ DECL_WRITER(int32_t);<br>
+ DECL_WRITER(int64_t);<br>
+ DECL_WRITER(uint8_t);<br>
+ DECL_WRITER(uint32_t);<br>
+<br>
+ void write_bool(bool data)<br>
+ {<br>
+ uint8_t val = data;<br>
+ write_uint8_t(val);<br>
+ }<br>
+<br>
+ /* write function that reallocates more memory if required */<br>
+ void write(const void *data, int size)<br>
+ {<br>
+ if (!memory || pos > (curr_size - size))<br>
+ if (!grow(size))<br>
+ return;<br>
+<br>
+ memcpy(memory + pos, data, size);<br>
+<br>
+ pos += size;<br>
+ }<br>
+<br>
+ void overwrite(const void *data, int size, int offset)<br>
+ {<br>
+ if (offset < 0 || offset + size > pos)<br>
+ return;<br>
+ memcpy(memory + offset, data, size);<br>
+ }<br>
+<br>
+ /* length is written to make reading safe */<br>
+ void write_string(const char *str)<br>
+ {<br>
+ uint32_t len = str ? strlen(str) : 0;<br>
+ write_uint32_t(len);<br>
+<br>
+ if (str)<br>
+ write(str, len + 1);<br>
+ }<br>
+<br>
+ unsigned position()<br>
+ {<br>
+ return pos;<br>
+ }<br>
+<br>
+ /**<br>
+ * check if some data was written<br>
+ */<br>
+ bool data_was_written(void *data, uint32_t id)<br>
+ {<br>
+ hash_entry *entry =<br>
+ _mesa_hash_table_search(data_hash, hash_value,<br>
+ (void*) (intptr_t) id);<br></blockquote><div><br></div><div>This isn't an efficient way to use a hashtable. You're always passing the same hash_value (the one computed in the constructor), which means that every single hash entry will be stored in the same chain, and lookups will be really slow.<br>
<br>Since you're only using this hashtable to detect duplicate glsl_types, there's a much easier approach: take advantage of the fact that glsl_type is a flyweight class (in other words, code elsewhere in Mesa ensures that no duplicate glsl_type objects exist anywhere in memory, so any two glsl_type pointers can be compared simply using pointer equality). Because of that, you should be able to just use a pointer-based hashtable, like ir_variable_refcount_visitor does. In short, that means:<br>
<br></div><div>- Get rid of the hash_value class member.<br><br></div><div>- In the constructor, do this: _mesa_hash_table_create(NULL, _mesa_key_pointer_equal)<br><br></div><div>- The data_was_written() function can just look like this:<br>
<br> bool data_was_written(void *data)<br> {<br> return _mesa_hash_table_search(data_hash, _mesa_hash_pointer(data),<br> data) != NULL;<br> }<br><br></div><div>- The mark_data_written() function can just look like this:<br>
</div><div><br></div><div> void mark_data_written(void *data)<br> {<br> _mesa_hash_table_insert(data_hash, _mesa_hash_pointer(data),<br> data, NULL);<br> }<br><br>Note that I'm passing NULL for the last argument of _mesa_hash_table_insert because we don't actually need the hashtable to act as a key/value store; we are just using it to check whether we've seen a pointer before; so when we're marking that we've seen a pointer we don't need to store any data with it. <br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ if (entry && entry->data == data)<br>
+ return true;<br>
+<br>
+ return false;<br>
+ }<br>
+<br>
+ /* mark that some data was written */<br>
+ void mark_data_written(void *data, uint32_t id)<br>
+ {<br>
+ _mesa_hash_table_insert(data_hash, hash_value,<br>
+ (void*) (intptr_t) id, data);<br>
+ }<br>
+<br>
+private:<br>
+<br>
+ /* reallocate more memory */<br>
+ bool grow(int size)<br>
+ {<br>
+ unsigned new_size = 2 * (curr_size + size);<br>
+ char *more_mem = (char *) realloc(memory, new_size);<br>
+ if (more_mem == NULL) {<br>
+ free(memory);<br>
+ memory = NULL;<br>
+ return false;<br>
+ } else {<br>
+ memory = more_mem;<br>
+ curr_size = new_size;<br>
+ return true;<br>
+ }<br>
+ }<br>
+<br>
+ /* allocated memory */<br>
+ char *memory;<br>
+<br>
+ /* current size of the whole allocation */<br>
+ int curr_size;<br>
+<br>
+ /* write position / size of the data written */<br>
+ int pos;<br>
+<br>
+ /* this hash can be used to refer to data already written<br>
+ * to skip sequential writes of the same data<br>
+ */<br>
+ struct hash_table *data_hash;<br>
+ uint32_t hash_value;<br>
+<br>
+ static bool int_equal(const void *a, const void *b)<br>
+ {<br>
+ return a == b;<br>
+ }<br>
+<br>
+};<br>
+<br>
+#endif /* ifdef __cplusplus */<br>
+<br>
+#endif /* MEMORY_WRITER_H */<br>
<span class=""><font color="#888888">--<br>
1.8.5.3<br>
<br>
</font></span></blockquote></div><br></div></div></div>