Add support for reading bloom filter chunks#7279
Conversation
AHSauge
force-pushed
the
feature/read-bloom-filter
branch
from
a38d825 to
64da1ec
Compare
ethomson
left a comment
ethomson
left a comment
There was a problem hiding this comment.
Thanks for this! A few stylistic comments - the only really relevant one is pulling the structure definition into commit_graph.c
There was a problem hiding this comment.
At this point, I'd just pull these all together...
| return error; | |
| if ((error = commit_graph_parse_oid_fanout(file, data, &chunk_oid_fanout)) < 0 || | |
| (error = commit_graph_parse_oid_lookup(file, data, &chunk_oid_lookup)) < 0 || | |
| (error = commit_graph_parse_commit_data(file, data, &chunk_commit_data)) < 0 || | |
| (error = commit_graph_parse_extra_edge_list(file, data, &chunk_extra_edge_list)) < 0 || | |
| (error = commit_graph_parse_bloom_filter(file, data, | |
| &chunk_bloom_filter_index, &chunk_bloom_filter_data)) < 0) | |
| return error; | |
| @@ -59,6 +60,9 @@ typedef struct git_commit_graph_file { | |||
| /* The number of entries in the Extra Edge List table. Each entry is 4 bytes wide. */ | |||
| size_t num_extra_edge_list; | |||
|
|
|||
| /* Bloom filter data*/ | |||
There was a problem hiding this comment.
| /* Bloom filter data*/ | |
| /* Bloom filter data */ |
| if (bloom_filter_index->length == 0 | ||
| || bloom_filter_data->length < 12) |
There was a problem hiding this comment.
| if (bloom_filter_index->length == 0 | |
| || bloom_filter_data->length < 12) | |
| if (bloom_filter_index->length == 0 || | |
| bloom_filter_data->length < 12) |
| if (bloom_filter_index->length != file->num_commits * sizeof(uint32_t)) | ||
| return commit_graph_error("malformed Bloom Filter Index chunk"); |
There was a problem hiding this comment.
| if (bloom_filter_index->length != file->num_commits * sizeof(uint32_t)) | |
| return commit_graph_error("malformed Bloom Filter Index chunk"); | |
| if (bloom_filter_index->length != file->num_commits * sizeof(uint32_t)) | |
| return commit_graph_error("malformed Bloom Filter Index chunk"); |
| /* | ||
| * Current valid hash version is 1 or 2 | ||
| */ |
There was a problem hiding this comment.
| /* | |
| * Current valid hash version is 1 or 2 | |
| */ |
| * Current valid hash version is 1 or 2 | ||
| */ | ||
| hash_version = ntohl(data_header[0]); | ||
| if (hash_version < 1 || hash_version > 2) |
There was a problem hiding this comment.
| if (hash_version < 1 || hash_version > 2) | |
| if (hash_version < 1 || hash_version > 2) |
There was a problem hiding this comment.
This isn't a general-purpose bloom filter structure - I think that we should pull this into commit_graph.c with the rest of the commit graph structures.
There was a problem hiding this comment.
Sure. The rationale behind the split was just to allow for code isolation down the line, but we can address that if necessary when we get there. I've merged the structure into git_commit_graph_file since that seems more sensible, and I believe I've address the other comments you had as well.
Please let me know if you want further adjustments
AHSauge
force-pushed
the
feature/read-bloom-filter
branch
from
64da1ec to
0d1218c
Compare
AHSauge
force-pushed
the
feature/read-bloom-filter
branch
from
0d1218c to
dc6ffe0
Compare
2 participants
The filter settings are split out as a separate struct to make it easier to
avoid circular dependencies later down the line.
Related to #5758