Change the DNMD APIs to act on single rows and bring other perf impro…

…vements from the CoreCLR + DNMD experiment to DNMD.

Port of AaronRobinsonMSFT/DNMD#55

src/native/dnmd/src/dnmd/access.c

@@ -1,8 +1,7 @@
 #include "internal.h"
 
-bool create_access_context(mdcursor_t* cursor, col_index_t col_idx, uint32_t row_count, bool make_writable, access_cxt_t* acxt)
+bool create_access_context(mdcursor_t* cursor, col_index_t col_idx, bool make_writable, access_cxt_t* acxt)
 {
-    assert(acxt != NULL);
     mdtable_t* table = CursorTable(cursor);
     if (table == NULL)
         return false;
@@ -16,22 +15,90 @@ bool create_access_context(mdcursor_t* cursor, col_index_t col_idx, uint32_t row
 
     // Metadata row indexing is 1-based.
     row--;
-    acxt->table = table;
-    acxt->col_details = table->column_details[idx];
 
-    // Compute the offset into the first row.
-    uint32_t offset = ExtractOffset(acxt->col_details);
+    mdtcol_t col = table->column_details[idx];
 
+    uint32_t offset_to_table_data = row * table->row_size_bytes + ExtractOffset(col);
+#ifndef NDEBUG
+    size_t len = (col & mdtc_b2) ? 2 : 4;
+    assert(offset_to_table_data + len <= table->data.size);
+#endif
+
+    acxt->table = table;
+    acxt->data = table->data.ptr + offset_to_table_data;
+    acxt->col_details = col;
     if (make_writable)
     {
-        acxt->writable_data = get_writable_table_data(table, make_writable);
-        acxt->writable_data = acxt->writable_data + (row * table->row_size_bytes) + offset;
+        acxt->writable_data = get_writable_table_data(table, make_writable) + offset_to_table_data;
     }
     else
     {
         acxt->writable_data = NULL;
     }
-
+    return true;
+}
+
+bool read_column_data(access_cxt_t* acxt, uint32_t* data)
+{
+    assert(acxt != NULL && acxt->data != NULL && data != NULL);
+
+    uint8_t const* table_data = acxt->data;
+
+    if ((acxt->col_details & mdtc_b4) == mdtc_b4)
+    {
+        size_t len = 4;
+        return read_u32(&table_data, &len, data);
+    }
+    else
+    {
+        size_t len = 2;
+        uint16_t value;
+        if (!read_u16(&table_data, &len, &value))
+            return false;
+
+        *data = value;
+        return true;
+    }
+}
+
+bool write_column_data(access_cxt_t* acxt, uint32_t data)
+{
+    assert(acxt != NULL && acxt->writable_data != NULL);
+    uint8_t* table_data = acxt->writable_data;
+    if ((acxt->col_details & mdtc_b4) == mdtc_b4)
+    {
+        size_t len = 4;
+        return write_u32(&table_data, &len, data);
+    }
+    else
+    {
+        size_t len = 2;
+        return write_u16(&table_data, &len, (uint16_t)data);
+    }
+}
+
+bool create_bulk_access_context(mdcursor_t* cursor, col_index_t col_idx, uint32_t row_count, bulk_access_cxt_t* acxt)
+{
+    assert(acxt != NULL);
+    mdtable_t* table = CursorTable(cursor);
+    if (table == NULL)
+        return false;
+
+    uint32_t row = CursorRow(cursor);
+    if (row == 0 || row > table->row_count)
+        return false;
+
+    uint8_t idx = col_to_index(col_idx, table);
+    assert(idx < table->column_count);
+
+    // Metadata row indexing is 1-based.
+    row--;
+    acxt->table = table;
+    acxt->col_details = table->column_details[idx];
+
+    // Compute the offset into the first row.
+    uint32_t offset = ExtractOffset(acxt->col_details);
+
     acxt->start = acxt->data = table->data.ptr + (row * table->row_size_bytes) + offset;
 
     // Compute the beginning of the row after the last valid row.
@@ -50,40 +117,33 @@ bool create_access_context(mdcursor_t* cursor, col_index_t col_idx, uint32_t row
     return true;
 }
 
-bool read_column_data(access_cxt_t* acxt, uint32_t* data)
+bool read_column_data_and_advance(bulk_access_cxt_t* acxt, uint32_t* data)
 {
     assert(acxt != NULL && data != NULL);
     *data = 0;
 
-    if (acxt->writable_data != NULL)
-        acxt->writable_data += (acxt->col_details & mdtc_b2) ? 2 : 4;
-
-    return (acxt->col_details & mdtc_b2)
-        ? read_u16(&acxt->data, &acxt->data_len, (uint16_t*)data)
-        : read_u32(&acxt->data, &acxt->data_len, data);
-}
-
-bool write_column_data(access_cxt_t* acxt, uint32_t data)
-{
-    assert(acxt != NULL && acxt->writable_data != NULL);
-
-    acxt->data += (acxt->col_details & mdtc_b2) ? 2 : 4;
+    if ((acxt->col_details & mdtc_b4) == mdtc_b4)
+    {
+        return read_u32(&acxt->data, &acxt->data_len, data);
+    }
+    else
+    {
+        uint16_t value;
+        if (!read_u16(&acxt->data, &acxt->data_len, &value))
+            return false;
 
-    return (acxt->col_details & mdtc_b2)
-        ? write_u16(&acxt->writable_data, &acxt->data_len, (uint16_t)data)
-        : write_u32(&acxt->writable_data, &acxt->data_len, data);
+        *data = value;
+        return true;
+    }
 }
 
-bool next_row(access_cxt_t* acxt)
+bool next_row(bulk_access_cxt_t* acxt)
 {
     assert(acxt != NULL);
     // We will only traverse correctly if we've already read the column in this row.
     assert(acxt->data_len == 0);
     acxt->data += acxt->next_row_stride;
 
-    if (acxt->writable_data != NULL)
-        acxt->writable_data += acxt->next_row_stride;
-
     // Restore the data length of the column data.
     acxt->data_len = acxt->data_len_col;
     return acxt->data < acxt->end;

src/native/dnmd/src/dnmd/bytes.c

@@ -129,6 +129,9 @@ bool read_i8(uint8_t const** data, size_t* data_len, int8_t* o)
     return read_le(data, data_len, o, sizeof(*o));
 }
 
+// MSVC doesn't optimize away the implementation on Little-Endian platforms,
+// so manually provide an optimized implementation for MSVC.
+#ifndef _MSC_VER
 bool read_u16(uint8_t const** data, size_t* data_len, uint16_t* o)
 {
     return read_le(data, data_len, o, sizeof(*o));
@@ -159,6 +162,75 @@ bool read_i64(uint8_t const** data, size_t* data_len, int64_t* o)
     return read_le(data, data_len, o, sizeof(*o));
 }
 
+#else
+bool read_u16(uint8_t const** data, size_t* data_len, uint16_t* o)
+{
+    if (*data_len < sizeof(*o))
+        return false;
+
+    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 < sizeof(*o))
+        return false;
+
+    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 < sizeof(*o))
+        return false;
+
+    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 < sizeof(*o))
+        return false;
+
+    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 < sizeof(*o))
+        return false;
+
+    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 < sizeof(*o))
+        return false;
+
+    memcpy(o, *data, sizeof(*o));
+    *data += sizeof(*o);
+    *data_len -= sizeof(*o);
+    return true;
+}
+
+#endif
+
 bool write_u8(uint8_t** data, size_t* data_len, uint8_t o)
 {
     return write_le(data, data_len, o, sizeof(o));
@@ -208,35 +280,42 @@ bool decompress_u32(uint8_t const** data, size_t* data_len, uint32_t* o)
     assert(s != NULL);
 
     uint32_t val;
-    switch (*s & 0xc0)
+
+    // The valid leading bits are 00, 10, and 110.
+    // All others are invalid.
+    // PERF: Check for 00 vs 10 first as we get better codegen
+    // on Intel/AMD processors (shorter instruction sequences and better branch prediction).
+    if ((*s & 0x80) == 0x00)
     {
-    case 0xc0:
-        if (*data_len < 4)
+        if (*data_len < 1)
             return false;
 
-        *data_len -= 4;
-        val = ((*s++ & 0x1f) << 24);
-        val |= (*s++ << 16);
-        val |= (*s++ << 8);
-        val |= *s++;
-        break;
-
-    case 0x80:
+        *data_len -= 1;
+        val = *s++;
+    }
+    else if ((*s & 0xC0) == 0x80)
+    {
         if (*data_len < 2)
             return false;
 
         *data_len -= 2;
         val = ((*s++ & 0x3f) << 8);
         val |= *s++;
-        break;
-
-    default:
-        if (*data_len < 1)
+    }
+    else if ((*s & 0xE0) == 0xC0)
+    {
+        if (*data_len < 4)
             return false;
 
-        *data_len -= 1;
-        val = *s++;
-        break;
+        *data_len -= 4;
+        val = ((*s++ & 0x1f) << 24);
+        val |= (*s++ << 16);
+        val |= (*s++ << 8);
+        val |= *s++;
+    }
+    else
+    {
+        return false;
     }
 
     *o = val;

src/native/dnmd/src/dnmd/deltas.c

@@ -69,7 +69,7 @@ static bool initialize_token_map(mdtable_t* map, enc_token_map_t* token_map)
     for (uint32_t i = 0; i < map->row_count; (void)md_cursor_next(&map_cur), ++i)
     {
         mdToken tk;
-        if (1 != md_get_column_value_as_constant(map_cur, mdtENCMap_Token, 1, &tk))
+        if (!md_get_column_value_as_constant(map_cur, mdtENCMap_Token, &tk))
             return false;
 
         mdtable_id_t table_id = ExtractTokenType(RemoveRecordBit(tk));
@@ -122,7 +122,7 @@ static bool resolve_token(enc_token_map_t* token_map, mdToken referenced_token,
     for (uint32_t i = 0; i < token_map->map_cur_by_table[type].count; md_cursor_next(&map_record), i++)
     {
         mdToken mappedToken;
-        if (1 != md_get_column_value_as_constant(map_record, mdtENCMap_Token, 1, &mappedToken))
+        if (!md_get_column_value_as_constant(map_record, mdtENCMap_Token, &mappedToken))
             return false;
 
         assert((mdtable_id_t)ExtractTokenType(RemoveRecordBit(mappedToken)) == type);
@@ -165,8 +165,8 @@ static bool process_log(mdcxt_t* cxt, mdcxt_t* delta)
     {
         mdToken tk;
         uint32_t op;
-        if (1 != md_get_column_value_as_constant(log_cur, mdtENCLog_Token, 1, &tk)
-            || 1 != md_get_column_value_as_constant(log_cur, mdtENCLog_Op, 1, &op))
+        if (!md_get_column_value_as_constant(log_cur, mdtENCLog_Token, &tk)
+            || !md_get_column_value_as_constant(log_cur, mdtENCLog_Op, &op))
         {
             return false;
         }
@@ -343,11 +343,11 @@ bool merge_in_delta(mdcxt_t* cxt, mdcxt_t* delta)
     mdcursor_t delta_module = create_cursor(&delta->tables[mdtid_Module], 1);
 
     mdguid_t base_mvid;
-    if (1 != md_get_column_value_as_guid(base_module, mdtModule_Mvid, 1, &base_mvid))
+    if (!md_get_column_value_as_guid(base_module, mdtModule_Mvid, &base_mvid))
         return false;
 
     mdguid_t delta_mvid;
-    if (1 != md_get_column_value_as_guid(delta_module, mdtModule_Mvid, 1, &delta_mvid))
+    if (!md_get_column_value_as_guid(delta_module, mdtModule_Mvid, &delta_mvid))
         return false;
 
     // MVIDs must match between base and delta images.
@@ -358,8 +358,8 @@ bool merge_in_delta(mdcxt_t* cxt, mdcxt_t* delta)
     // This ensures that we are applying deltas in order.
     mdguid_t enc_id;
     mdguid_t delta_enc_base_id;
-    if (1 != md_get_column_value_as_guid(base_module, mdtModule_EncId, 1, &enc_id)
-        || 1 != md_get_column_value_as_guid(delta_module, mdtModule_EncBaseId, 1, &delta_enc_base_id)
+    if (!md_get_column_value_as_guid(base_module, mdtModule_EncId, &enc_id)
+        || !md_get_column_value_as_guid(delta_module, mdtModule_EncBaseId, &delta_enc_base_id)
         || memcmp(&enc_id, &delta_enc_base_id, sizeof(mdguid_t)) != 0)
     {
         return false;
@@ -382,10 +382,10 @@ bool merge_in_delta(mdcxt_t* cxt, mdcxt_t* delta)
     // We don't want to manipulate the heap sizes, so we'll pull the heap offset directly from the delta and use that
     // in the base image.
     uint32_t new_enc_base_id_offset;
-    if (1 != get_column_value_as_heap_offset(delta_module, mdtModule_EncId, 1, &new_enc_base_id_offset))
+    if (!get_column_value_as_heap_offset(delta_module, mdtModule_EncId, &new_enc_base_id_offset))
         return false;
-    if (1 != set_column_value_as_heap_offset(base_module, mdtModule_EncId, 1, &new_enc_base_id_offset))
+    if (!set_column_value_as_heap_offset(base_module, mdtModule_EncId, new_enc_base_id_offset))
         return false;
 
     return true;
-}
+}