Convert shared string table from hash to RB tree. #41

Merged
merged 1 commit into from Dec 11, 2015

Conversation

Projects
None yet
2 participants
@mrenters
Contributor

mrenters commented Dec 11, 2015

Resubmit on dev branch. This speeds up performance of there shared string table with many shared strings.

+ struct sst_element *element2);
+
+LXW_RB_GENERATE_ELEMENT(sst_rb_tree, sst_element, sst_tree_pointers,
+ _element_cmp);

This comment has been minimized.

@jmcnamara

jmcnamara Dec 11, 2015

Owner

We could probably just pass strcmp() here directly without a wrapper, right?

@jmcnamara

jmcnamara Dec 11, 2015

Owner

We could probably just pass strcmp() here directly without a wrapper, right?

This comment has been minimized.

@mrenters

mrenters Dec 11, 2015

Contributor

You can’t as the first thing in the struct is the index, not the string. The strcmp compares element->string. strcmp also expects a char * and not an element *.

@mrenters

mrenters Dec 11, 2015

Contributor

You can’t as the first thing in the struct is the index, not the string. The strcmp compares element->string. strcmp also expects a char * and not an element *.

This comment has been minimized.

@jmcnamara

jmcnamara Dec 11, 2015

Owner

Ah yes. Of course.

@jmcnamara

jmcnamara Dec 11, 2015

Owner

Ah yes. Of course.

- /* Store the number of buckets to calculate the load factor. */
- sst->num_buckets = NUM_SST_BUCKETS;
+ /* Initialize the RB tree. */
+ RB_INIT(sst->rb_tree);
return sst;
mem_error2:

This comment has been minimized.

@jmcnamara

jmcnamara Dec 11, 2015

Owner

Not your issue but I wonder if we still need 2 mem_error labels since free(sst) should take care of it. You could fix that in passing or I can look into it.

@jmcnamara

jmcnamara Dec 11, 2015

Owner

Not your issue but I wonder if we still need 2 mem_error labels since free(sst) should take care of it. You could fix that in passing or I can look into it.

This comment has been minimized.

@jmcnamara

jmcnamara Dec 11, 2015

Owner

Ignore. I'll look into it after merge.

@jmcnamara

jmcnamara Dec 11, 2015

Owner

Ignore. I'll look into it after merge.

-struct sst_bucket_list {
- struct sst_element *slh_first;
-};
+/* Define a tree.h RB structure for storing shared strings. */

This comment has been minimized.

@jmcnamara

jmcnamara Dec 11, 2015

Owner

Overall this is much nicer, much cleaner.

@jmcnamara

jmcnamara Dec 11, 2015

Owner

Overall this is much nicer, much cleaner.

jmcnamara added a commit that referenced this pull request Dec 11, 2015

Merge pull request #41 from mrenters/rb_sst
Convert shared string table from hash to RB tree.

@jmcnamara jmcnamara merged commit bee520b into jmcnamara:dev Dec 11, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Dec 11, 2015

Owner

Merged. Thanks.

Owner

jmcnamara commented Dec 11, 2015

Merged. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment