Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jkoritzinsky committed Oct 11, 2024
1 parent 88867c2 commit 3199d55
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 73 deletions.
1 change: 0 additions & 1 deletion src/dnmd/access.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ bool create_access_context(mdcursor_t* cursor, col_index_t col_idx, bool make_wr
bool read_column_data(access_cxt_t* acxt, uint32_t* data)
{
assert(acxt != NULL && acxt->data != NULL && data != NULL);
*data = 0;

uint8_t const* table_data = acxt->data;

Expand Down
48 changes: 24 additions & 24 deletions src/dnmd/bytes.c
Original file line number Diff line number Diff line change
Expand Up @@ -165,67 +165,67 @@ bool read_i64(uint8_t const** data, size_t* data_len, int64_t* o)
#else
bool read_u16(uint8_t const** data, size_t* data_len, uint16_t* o)
{
if (*data_len < 2)
if (*data_len < sizeof(*o))
return false;

memcpy(o, *data, 2);
*data += 2;
*data_len -= 2;
memcpy(o, *data, sizeof(*o));
*data += sizeof(*o);
*data_len -= sizeof(*o);
return true;
}

bool read_i16(uint8_t const** data, size_t* data_len, int16_t* o)
{
if (*data_len < 2)
if (*data_len < sizeof(*o))
return false;

memcpy(o, *data, 2);
*data += 2;
*data_len -= 2;
memcpy(o, *data, sizeof(*o));
*data += sizeof(*o);
*data_len -= sizeof(*o);
return true;
}

bool read_u32(uint8_t const** data, size_t* data_len, uint32_t* o)
{
if (*data_len < 4)
if (*data_len < sizeof(*o))
return false;

memcpy(o, *data, 4);
*data += 4;
*data_len -= 4;
memcpy(o, *data, sizeof(*o));
*data += sizeof(*o);
*data_len -= sizeof(*o);
return true;
}

bool read_i32(uint8_t const** data, size_t* data_len, int32_t* o)
{
if (*data_len < 4)
if (*data_len < sizeof(*o))
return false;

memcpy(o, *data, 4);
*data += 4;
*data_len -= 4;
memcpy(o, *data, sizeof(*o));
*data += sizeof(*o);
*data_len -= sizeof(*o);
return true;
}

bool read_u64(uint8_t const** data, size_t* data_len, uint64_t* o)
{
if (*data_len < 8)
if (*data_len < sizeof(*o))
return false;

memcpy(o, *data, 8);
*data += 8;
*data_len -= 8;
memcpy(o, *data, sizeof(*o));
*data += sizeof(*o);
*data_len -= sizeof(*o);
return true;
}

bool read_i64(uint8_t const** data, size_t* data_len, int64_t* o)
{
if (*data_len < 8)
if (*data_len < sizeof(*o))
return false;

memcpy(o, *data, 8);
*data += 8;
*data_len -= 8;
memcpy(o, *data, sizeof(*o));
*data += sizeof(*o);
*data_len -= sizeof(*o);
return true;
}

Expand Down
51 changes: 4 additions & 47 deletions src/dnmd/query.c
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,6 @@ bool get_column_value_as_heap_offset(mdcursor_t c, col_index_t col_idx, uint32_t
if (heap == NULL)
return false;

#ifdef DEBUG_COLUMN_SORTING
validate_column_not_sorted(acxt.table, col_idx);
#endif

if (!read_column_data(&acxt, offset))
return false;

Expand All @@ -321,17 +317,7 @@ bool md_get_column_value_as_utf8(mdcursor_t c, col_index_t col_idx, char const**
if (!(acxt.col_details & mdtc_hstring))
return false;

mdstream_t const* heap = &
acxt.table->cxt->strings_heap;
if (heap == NULL)
return false;

#ifdef DEBUG_COLUMN_SORTING
validate_column_not_sorted(acxt.table, col_idx);
#endif

uint32_t offset;

if (!read_column_data(&acxt, &offset))
return false;

Expand All @@ -353,17 +339,7 @@ bool md_get_column_value_as_userstring(mdcursor_t c, col_index_t col_idx, mduser
if (!(acxt.col_details & mdtc_hus))
return false;

mdstream_t const* heap = &
acxt.table->cxt->strings_heap;
if (heap == NULL)
return false;

#ifdef DEBUG_COLUMN_SORTING
validate_column_not_sorted(acxt.table, col_idx);
#endif

uint32_t offset;

if (!read_column_data(&acxt, &offset))
return false;

Expand All @@ -386,17 +362,7 @@ bool md_get_column_value_as_blob(mdcursor_t c, col_index_t col_idx, uint8_t cons
if (!(acxt.col_details & mdtc_hblob))
return false;

mdstream_t const* heap = &
acxt.table->cxt->strings_heap;
if (heap == NULL)
return false;

#ifdef DEBUG_COLUMN_SORTING
validate_column_not_sorted(acxt.table, col_idx);
#endif

uint32_t offset;

if (!read_column_data(&acxt, &offset))
return false;

Expand All @@ -418,17 +384,7 @@ bool md_get_column_value_as_guid(mdcursor_t c, col_index_t col_idx, mdguid_t* gu
if (!(acxt.col_details & mdtc_hguid))
return false;

mdstream_t const* heap = &
acxt.table->cxt->strings_heap;
if (heap == NULL)
return false;

#ifdef DEBUG_COLUMN_SORTING
validate_column_not_sorted(acxt.table, col_idx);
#endif

uint32_t offset;

if (!read_column_data(&acxt, &offset))
return false;

Expand Down Expand Up @@ -484,7 +440,6 @@ int32_t md_get_many_rows_column_value_as_token(mdcursor_t c, uint32_t col_idx, u
return read_in;
}


bool md_get_column_values_raw(mdcursor_t c, uint32_t values_length, bool* values_to_get, uint32_t* values_raw)
{
if (values_length > 0
Expand Down Expand Up @@ -549,7 +504,8 @@ static int32_t col_compare_2bytes(void const* key, void const* row, void* cxt)
assert(success && col_len == 0);
(void)success;

return lhs - rhs;
// 0: equal, < 0: (lhs < rhs), > 0: (lhr > rhs)
return lhs - rhs;
}

static int32_t col_compare_4bytes(void const* key, void const* row, void* cxt)
Expand All @@ -567,7 +523,8 @@ static int32_t col_compare_4bytes(void const* key, void const* row, void* cxt)
assert(success && col_len == 0);
(void)success;

return lhs - rhs;
// 0: equal, < 0: (lhs < rhs), > 0: (lhr > rhs)
return lhs - rhs;
}

typedef int32_t(*md_bcompare_t)(void const* key, void const* row, void*);
Expand Down
5 changes: 4 additions & 1 deletion src/dnmd/search.template.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#error Must define SEARCH_FUNC_NAME(name) macro
#endif // SEARCH_FUNC_NAME


static void const* SEARCH_FUNC_NAME(md_lsearch)(
void const* key,
void const* base,
Expand Down Expand Up @@ -43,6 +42,10 @@ static void const* SEARCH_FUNC_NAME(md_bsearch)(
void* cxt)
{
assert(key != NULL && base != NULL);
// PERF: Do a binary search until we have at most 16 elements.
// Then switch to a linear search. A linear search has better cache locality
// and better branch prediction and can end up being faster than a binary search
// for small data sets.
while (count > 16)
{
void const* row = (uint8_t const*)base + (element_size * (count / 2));
Expand Down

0 comments on commit 3199d55

Please sign in to comment.