-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge: write output to block files #34
Conversation
@@ -0,0 +1,186 @@ | |||
#include "zebra_append.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just splitting files, you can ignore it
return ZEBRA_SUCCESS; | ||
} | ||
|
||
error_t zebra_append_block_entity (anemone_mempool_t *pool, zebra_entity_t *entity, zebra_block_t **inout_block) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is new code
@@ -0,0 +1,189 @@ | |||
#include "zebra_block_split.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just splitting files, you can ignore it
|
||
in_table->row_count -= n_rows; | ||
out_table->row_count = n_rows; | ||
out_table->row_capacity = n_rows; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except for adding this line
@@ -0,0 +1,214 @@ | |||
#include "zebra_clone.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just splitting files, you can ignore it
into_data->d = ZEBRA_CLONE_ARRAY (pool, table_data->d, row_capacity ); | ||
break; | ||
case ZEBRA_ARRAY: | ||
into_data->a.n = ZEBRA_CLONE_ARRAY (pool, table_data->a.n, row_capacity ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except for adding this line
@@ -0,0 +1,105 @@ | |||
#include "zebra_grow.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just splitting files, you can ignore it
@@ -0,0 +1,102 @@ | |||
#ifndef __ZEBRA_DEBUG_H |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unused shit, you can ignore it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's cool if you want to keep this, looks like it might be useful in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I'm pretty sure it'll be useful
return ZEBRA_MERGE_DIFFERENT_ENTITIES; | ||
} | ||
|
||
block->entities = ZEBRA_GROW_ARRAY (pool, block->entities, block->entity_count, block->entity_count + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it right that we're reallocating here for every entity? perhaps we should track capacity for this array so we can do this more efficiently
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - I don't think we'd even need to track capacity since we can compute capacity based on current count. I just wanted the simplest thing possible, and I don't think this is worth spending time on while the read/write and convert is so slow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in a profile I just ran, appendEntityToBlock
(which calls this function) has 0% of time. it would be adding extra pressure on the mempool but I think we have bigger issues.
I'd rather just add this as a TODO or issue so we don't forget about it, and move on to other bits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good to me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added issue #35
block->times = ZEBRA_GROW_ARRAY (pool, block->times, block->row_count, new_row_count); | ||
block->priorities = ZEBRA_GROW_ARRAY (pool, block->priorities, block->row_count, new_row_count); | ||
block->tombstones = ZEBRA_GROW_ARRAY (pool, block->tombstones, block->row_count, new_row_count); | ||
block->row_count = new_row_count; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same deal here, I guess the issue is that zebra_block_t
wasn't designed with mutation in mind, but i don't see a problem with bolting on some capacity fields
block = anemone_mempool_calloc (pool, 1, sizeof (zebra_block_t) ); | ||
} else if (entity->attribute_count != block->table_count) { | ||
// TODO: better error | ||
return ZEBRA_MERGE_DIFFERENT_ENTITIES; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well fix this TODO
outfd <- lift $ IO.openBinaryFile out IO.WriteMode | ||
lift $ Builder.hPutBuilder outfd (Serial.bHeader fileheader) | ||
|
||
pool0 <- lift $ Mempool.create |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the benefits of executables? 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - actually I guess I can free this. I don't have the same problem as #31 since it's in an IORef
firstT fromForeignError . foreignEntitiesOfBlock' | ||
|
||
prop_c_block_of_entities :: Property | ||
prop_c_block_of_entities = | ||
gamble (noShrink jBlock) $ check_block_of_entities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we remove noShrink
now it's working?
this is good btw 👍 |
… check so error is obvious
53629a9
to
1c95c7d
Compare
No description provided.