Hi, Vladislav! On May 09, Vladislav Vaintroub wrote:
+ void set_all() + { + set_prefix(width);
may be better to optimize it? memset(buffer, 0xff, sizeof(buffer) - 1); ((uchar*)buffer)[sizeof(buffer)-1] = last_byte_mask(width);
I’m not really convinced this would be an optimization 😊 My claim is that, since “width” is constant at compile time (128), and it is divisible by 32, any decent C++ compiler must be able to figure out, at compile time, that set_all() == set_prefix(width)==set_prefix(128) trivially reduces to a shorter
I hoped a compiler would optimize it, all constants are known at compile time, indeed. But I didn't quite trust it to do so :) But ok, whatever you prefer.
+ { + compile_time_assert(sizeof(ulonglong) == 8); + uint32 tmp[2]; + int8store(tmp, map2buff); + if (array_elements(buffer) > 2 && pad_with_ones) + set_all();
really? I'd expect that if pad_with_ones==true, you simply don't need to do anything to the rest of the buffer.
The original implementation does padding with 0xff or 0x00 , depending on the highest order bit of map2buff, with bitmap_set_above(). retained that property
Oh. Could you please add a comment about this method's semantics? Based just on the name, one can really expect that the Bitmap is intersected with "ulonglong map2buff argument, padded with 0's or 1's up to Bitmap's width". And even with that semantics set_all() looks wrong. It should only set the tail (bits above 64) to 1, not all bits. Regards, Sergei Chief Architect MariaDB and security@mariadb.org