Mats Kindahl wrote:
> In summary:
>
> - Overallocation of memory in blocks of 512 bytes
>
> - Thrashing memory accesses forcing caches to be flushed and loaded very
> often.
>
> - I think you would be better of using the attached implementation of a
> dynamic
> bitvector instead of bitset (I wrote it a few years ago), since that will
> just
> allocate the necessary memory.
Please note that the bitvector class allocate memory on the heap unless a custom
allocator is provided. You may or may not want this.
MY_BITMAP was used so that it allocated memory on the stack, if the bitvector
was small enough (approx. <120 bits, perhaps - memory for administration), which
automatically means there is no malloc() mutex used.
It is not difficult to write an allocator to do that job, so tell me if you feel
this is needed.
Just my few cents,
Mats Kindahl
>
> Just my few cents,
> Mats Kindahl
>
> ----------------------------
> This looks strange, without having checked the context, I would look closer
> at it.
>
> @@ -113,7 +113,7 @@
> assert(bitmap);
> if (result_field)
> {
> - bitmap->set(field->field_index);
> + bitmap->set(result_field->field_index);
> }
> return 0;
> }
>
>
> ----------------------------
> This allocates one temporary object of size 512 bytes (!) just to check if any
> bit is set. This will iterates over all 4096 fields, or all 512 bytes at least
> two times. If is_key_used() is mostly false, this will be a considerable
> impact
> on the execution time each time this function is called. Note that most tables
> do not have 4096 fields, but significantly less.
>
> -bool is_key_used(Table *table, uint32_t idx, const bitmap<MAX_FIELDS>
> *fields)
> +bool is_key_used(Table *table, uint32_t idx, const bitset<MAX_FIELDS>
> *fields)
> {
> table->tmp_set.reset();
> table->mark_columns_used_by_index_no_reset(idx, &table->tmp_set);
> - /* TODO: change this to use std::bitset */
> - if (bitmap_is_overlapping(&table->tmp_set, fields))
> + /* Check if 2 bitsets are overlapping */
> + bitset<MAX_FIELDS> tmp= *fields & table->tmp_set;
> + if (tmp.any())
> return 1;
>
> /*
>
>
> ----------------------------
> This will allocate two temporaries, each of size 512 bytes, and iterate over
> them to check if one bitset is a subset of the other.
>
> @@ -126,6 +127,19 @@
> return static_cast<ha_rows>(x);
> }
>
> +/*
> + * This helper function returns true if map1 is a subset of
> + * map2; otherwise it returns false.
> + */
> +static bool is_bitmap_subset(const bitset<MAX_FIELDS> *map1, const
> bitset<MAX_FIELDS> *map2)
> +{
> + bitset<MAX_FIELDS> tmp1= *map2;
> + tmp1.flip();
> + bitset<MAX_FIELDS> tmp2= *map1 & tmp1;
> + return (!tmp2.any());
> +}
> +
> +
> static int sel_cmp(Field *f,unsigned char *a,unsigned char *b,uint8_t
> a_flag,uint8_t b_flag);
>
> static unsigned char is_null_string[2]= {1,0};
>
> ------------------------------
> This will allocate two 512 bytes variables on the stack where the old
> implementation just allocates memory for the columns actually used.
>
> @@ -698,8 +712,8 @@
> bool quick; // Don't calulate possible keys
>
> uint32_t fields_bitmap_size;
> - MY_BITMAP needed_fields; /* bitmask of fields needed by the query */
> - MY_BITMAP tmp_covered_fields;
> + bitset<MAX_FIELDS> needed_fields; /* bitmask of fields needed by the query
> */
> + bitset<MAX_FIELDS> tmp_covered_fields;
>
> key_map *needed_reg; /* ptr to SQL_SELECT::needed_reg */
>
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Mailing list: https://launchpad.net/~drizzle-discuss
> Post to : [email protected]
> Unsubscribe : https://launchpad.net/~drizzle-discuss
> More help : https://help.launchpad.net/ListHelp
--
Mats Kindahl
Senior Software Engineer
Database Technology Group
Sun Microsystems
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help : https://help.launchpad.net/ListHelp
