Jay Pipes wrote:
> Further narrowing down has focused on the revision series r982.1.1
> through r982.1.5.  Attached is the diff for that, and somewhere in there
> is the regression.
> 
> Note that the goal here is not necessarily to undo Padraig's work on
> standardizing Bitset, but to look into why/where specifically we're
> seeing that regression on concurrency loads >128.  Remember, no shame,
> no blame, many hands make light work. :)
> 
> Eyeballs please! :)

OK, I have attached a some examples of pieces that look suspicious. They are all
focused on the working (memory) set of a thread and where it is bigger than it
is using the old bitmap structure.

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.

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 */

-- 
Mats Kindahl
Senior Software Engineer
Database Technology Group
Sun Microsystems
/* -*- Mode: C++ -*- */

/*
  Copyright 2005 Mats Kindahl. All rights reserverd.

  Redistribution and use in source and binary forms, with or without
  modification, are permitted provided that the following conditions
  are met:

  * Redistributions of source code must retain the above copyright
    notice, this list of conditions and the following disclaimer.

  * Redistributions in binary form must reproduce the above copyright
    notice, this list of conditions and the following disclaimer in
    the documentation and/or other materials provided with the
    distribution.

  * Neither the name of the copyright owner or other contributors may
    be used to endorse or promote products derived from this software
    without specific prior written permission.

  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
  FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
  COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
  BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
  LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
  ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
  POSSIBILITY OF SUCH DAMAGE.
*/


#ifndef BITVECTOR_H
#define BITVECTOR_H

#include <cstring>
#include <climits>
#include <algorithm>
#include <memory>
#include <utility>

/* Some compile-time checks to ensure the integrity of the implementation. */
#if CHAR_BIT != 8
#  error "This implementation is designed for 8-bit bytes!"
#endif

#ifndef __cplusplus
#  error "This is not a C header file, it's a C++ header file"
#endif

#if NO_EXCEPTIONS
#define THROW_0
#define BAD_ALLOC() do { return 0; } while (0)
#else
#define THROW_0   throw()
#define BAD_ALLOC() do { throw std::bad_alloc(); } while (0)
#endif

using std::min;

/**
  Simple access threading model.
*/
struct No_threading {
  struct Read_sentry {
    Read_sentry() { }
  };

  struct Write_sentry {
    Write_sentry() { }
    Write_sentry(const Read_sentry&) { }
  };
};


/**
   A run-time sized bitvector for storing bits.

   Right now, the vector cannot change size. It's only used as a
   replacement for using an array of bytes and a counter. If you want
   to change the size of the vector, construct a new bitvector and
   assign it to the vector, e.g.:

   @code
   bitvector new_bv(new_size);
   old_bv = new_bv;
   @endcode

   An alternative is to use the swap member function to replace the
   instance:

   @code
   bitvector new_bv(new_size);
   old_bv.swap(new_bv);
   @endcode

   @note This class is not designed to be inherited from, so do it at
   your own risk.
*/

template < class Allocator = std::allocator<unsigned char>,
           class ThreadingModel = No_threading >
class basic_bitvector
  : public ThreadingModel, private Allocator
{
  /**
     Is all bits set in the bitvector?
   */
  friend bool is_set_all(const basic_bitvector& bv) {
    size_type bytes = byte_size(bv.m_size);
    unsigned char mask = bv.last_byte_mask();
    typename ThreadingModel::Read_sentry sentry;
    for (size_type i = 0 ; i < bytes - 1; ++i)
      if (bv.m_ptr[i] != 0xFF)
        return false;
    if (bv.m_ptr[bytes - 1] & mask != mask)
      return false;
    return true;
  }

  /**
     Is any bit set in the bitvector?

     @param bv Bitvector to investigate

     @return @c true if at least one bit is set in the bitvector, @c
     false otherwise.
   */
  friend bool is_set_any(const basic_bitvector& bv) {
    size_type bytes = byte_size(bv.m_size);
    unsigned char mask = bv.last_byte_mask();
    typename ThreadingModel::Read_sentry sentry;
    for (size_type i = 0 ; i < bytes - 1; ++i)
      if (bv.m_ptr[i] != 0xFF)
        return false;
    if (bv.m_ptr[bytes - 1] & mask != mask)
      return false;
    return true;
  }

  /**
     Compute a complemented bitvector.

     Compute the complement of a bitvector by applying the @c ~
     operator on each byte of the bitvector.

     @return The complemented bitvector.
   */
  friend basic_bitvector operator~(basic_bitvector arg) {
    typename basic_bitvector::size_type const len = byte_size(arg.m_size);
    for (size_type i = 0 ; i < len ; ++i) {
      arg.m_ptr[i] = ~arg.m_ptr[i];
    }
    return arg;
  }


  /**
     Compute the <em>bitwise and</em> of two bitvectors.

     If the vectors have different sizes, the smallest of the vectors
     is assumed to have the remaining bits clear.

     @return A new bitvector that is the bitwise and of the two
     vectors. The new vector will have a size equal to the largest of
     the two vectors.
   */
  friend basic_bitvector
  operator&(basic_bitvector lhs, const basic_bitvector& rhs) {
    lhs &= rhs;
    return lhs;
  }


  /**
     Compute the <em>bitwise or</em> of two bitvectors.

     If the vectors have different sizes, the smallest of the vectors
     is assumed to have the remaining bits clear.

     @return A new bitvector that is the bitwise or of the two
     vectors. The new vector will have a size equal to the largest of
     the two vectors.
   */
  friend basic_bitvector
  operator|(basic_bitvector lhs, const basic_bitvector& rhs) {
    lhs |= rhs;
    return lhs;
  }


  /**
     Compute the <em>bitwise xor</em> of two bitvectors.

     If the vectors have different sizes, the smallest of the vectors
     is assumed to have the remaining bits clear.

     @return A new bitvector that is the bitwise xor of the two
     vectors. The new vector will have a size equal to the largest of
     the two vectors.
   */
  friend basic_bitvector
  operator^(basic_bitvector lhs, const basic_bitvector& rhs) {
    lhs ^= rhs;
    return lhs;
  }

private:
  typedef Allocator allocator_type;

  typedef typename ThreadingModel::Write_sentry    Write_sentry;
  typedef typename ThreadingModel::Read_sentry     Read_sentry;

  typedef typename allocator_type::pointer         pointer;
  typedef typename allocator_type::const_pointer   const_pointer;
  typedef typename allocator_type::value_type      value_type;
  typedef typename allocator_type::size_type       size_type;
  typedef typename allocator_type::difference_type difference_type;

  /* Helper classes */
  struct flip_bit_op {
    void operator()(pointer p, value_type m) { *p^= m; }
  };

  struct set_bit_op {
    void operator()(pointer p, value_type m) { *p |= m; }
  };

  struct clear_bit_op {
    void operator()(pointer p, value_type m) { *p &= ~m; }
  };

  struct test_bit_op {
    bool operator()(const_pointer p, value_type m) { return *p & m; }
  };

  /**
   * Compute a mask for the last byte.
   *
   * @return A mask with bits clear for the unused bits of the last
   * byte and set for the used bits of the last byte.
   */
  unsigned char last_byte_mask() const {
    unsigned int const used= 1U + ((size() - 1U) & ~(1UL << CHAR_BIT));
    return ((1U << used) - 1);
  }

  /**
   * Tidy the last byte (by clearing the unused bits) of the bitvector
   * to make comparison easy.
   *
   * @note This code is assuming that we're working with 8-bit bytes.
   *
   * @note For the last byte (if it exists) the lower unused bits are
   * clear. The bits within each byte is stored in big-endian
   * order.
   */
  void tidy_last_byte() {
    if (m_size > 0) {
      pointer const last_byte = m_ptr + bytes() - 1;
      *last_byte &= last_byte_mask();
    }
  }

  /**
     Helper function to apply an operation to a byte in the bitvector
     representation.
   */
  template <class ReturnType, class Sentry, class Func>
  ReturnType apply_to_byte(size_type const pos, Func op) const {
    Sentry sentry;
    /* Here I'm assuming that we're working with 8-bit bytes. */
    difference_type const byte_pos= pos >> 3;
    value_type const mask= (1 << (pos & 0x7U));
    return op(&m_ptr[byte_pos], mask);
  }


public:
  /** Default constructor. */
  basic_bitvector()
    : ThreadingModel(), Allocator(), m_size(0), m_ptr(0)
    {
    }

  /**
   * Constructor to construct a bitvector with a size.
   *
   * @param size The size of the bitvector in number of bits.
   * @param value The default value of the bits in the bitvector.
   * @param alloc The allocator to use when allocating memory.
   */
  explicit basic_bitvector(size_type size,
                           bool value = false,
                           allocator_type alloc = allocator_type())
    : ThreadingModel(), Allocator(alloc),
      m_size(size), m_ptr(allocate(byte_size(size)))
  {
    if (value)
      set_all();
    else
      clear_all();
  }

  /**
   * Constructor to create a bitvector from data.
   *
   * @param size Size of the bitvector in number of bits.
   * @param data Pointer to memory holding bytes for the bits. The
   *             bits are stored in little-endian order.
   * @param alloc Allocator to use when allocating memory.
   */
  explicit basic_bitvector(size_type size, const_pointer data,
                           allocator_type alloc = allocator_type())
    : ThreadingModel(), Allocator(alloc),
      m_size(size), m_ptr(allocate(byte_size(size)))
  {
    /* std::copy(data, data + byte_size(size), m_ptr); */
    memcpy(m_ptr, data, byte_size(size));
    tidy_last_byte();
  }

  /** Copy constructor */
  basic_bitvector(basic_bitvector const& other)
    : ThreadingModel(other), Allocator(other),
      m_size(other.size()), m_ptr(allocate(byte_size(other.size())))
    {
      memcpy(m_ptr, other.data(), other.bytes());
      tidy_last_byte();           /* Just a precaution */
    }

  /**
     Assignment operator.
   */
  basic_bitvector& operator=(basic_bitvector other) {
    swap(other);
    return *this;
  }

  /** Destructor */
  ~basic_bitvector() {
    deallocate(m_ptr, byte_size(m_size));
  }


  /**
   * Compute number of bytes required to store 'n' bits.
   */
  static inline size_type byte_size(size_type n) {
    size_type const byte_bits = sizeof(value_type) * CHAR_BIT;
    return (n + (byte_bits-1)) / byte_bits;
  }


  /**
     Swap the guts of this instance with another instance.

     @note It is only allowed to swap guts with another instance using
     the same type of allocator. Otherwise, the call to deallocate()
     will fail because it is using a different allocation model than
     the instance it swapped guts with.

     @todo Check if it can be allowed to swap guts with an instance
     with a different threading model.
  */
  void swap(basic_bitvector& other) {
    std::swap(m_size, other.m_size);
    std::swap(m_ptr, other.m_ptr);
  }

  /**
   * Get a pointer to the underlying bit store.
   *
   * @return A pointer to the memory where the underlying bits are
   * stored.
   */
  const_pointer data() const { return m_ptr; }


  /**
   * Get the size of the bitvector in bytes.
   *
   * The total amount of memory occupied by a bitvector
   * <code>bv</code> is <code>sizeof(bv) + bv.bytes()</code>.
   *
   * @return The number of bytes that the bits occupy.
   */
  size_type bytes() const { return byte_size(m_size); }


  /**
   * Get the size of the bitvector in number of bits.
   *
   * @return The number of bits in the bitvector.
   */
  size_type size() const { return m_size; }


  /**
   * Set all bits in the vector.
   */
  void set_all() {
    typename ThreadingModel::Write_sentry sentry;
    /* Here I'm assuming that we're working with 8-bit bytes. */
    memset(m_ptr, 255, bytes());
    tidy_last_byte();
  }


  /**
   * Set a bit in the bitfield.
   *
   * @param pos The bit to set, with 0 denoting the first bit.
   */
  void set_bit(size_t pos) {
    apply_to_byte<void,Write_sentry>(pos, set_bit_op());
  }

  /**
   * Reset (clear) all bits in the vector.
   */
  void clear_all() {
    typename ThreadingModel::Write_sentry sentry;
    memset(m_ptr, 0, bytes());
    tidy_last_byte();
  }

  /**
   * Reset (clear) one bit in the vector.
   *
   * @param pos The bit to reset, with 0 denoting the first bit.
   */
  void clear_bit(size_t pos) {
    apply_to_byte<void,Write_sentry>(pos, clear_bit_op());
  }

  /**
   * Flip one bit in the vector.
   *
   * @param pos The bit to flip, with 0 denoting the first bit.
   */
  void flip_bit(size_t pos) {
    apply_to_byte<void,Write_sentry>(pos, flip_bit_op());
  }

  /**
   * Get the value of a bit in the bitvector.
   *
   * @return @c true if the bit at position @c pos is set, @c false
   * otherwise.
   */
  bool get_bit(size_t pos) const {
    return apply_to_byte<bool,Read_sentry>(pos, test_bit_op());
  };

  /**
   * Compare two bitvectors for equality.
   *
   * Two bitvectors are equal if they have the same size and the bits
   * have the same values.
   *
   * @todo Change the semantics of the operator to match how the
   * bitwise operators work, that is: two bitvectors are equal if the
   * initial bits have the same values, and the extra bits of the
   * larger vector is all clear.
   *
   * @param rhs The right-hand side bitvector of the equality.
   *
   * @return @c true if the bitvectors are equal, @c false otherwise.
   */
  bool operator==(basic_bitvector const& rhs) const {
    if (size() != rhs.size())
      return false;

    // This works since I have ensured that the last byte of the
    // array contain sensible data.
    if (memcmp(data(), rhs.data(), bytes()) != 0)
      return false;
    return true;
  }

  /**
   * Compare two vectors for inequality.
   *
   * @return @c false if the bitvectors are equal, @c true otherwise.
   */
  bool operator!=(basic_bitvector const& rhs) const {
    return !(*this == rhs);
  }

  /**
   * In-place bitwise-or with another bitvector.
   *
   * The bitvector is modified to contain the result of computing the
   * bitwise-or of this bitvector with another bitvector. If the other
   * bitvector is larger, then the extra bits are ignored. If the
   * other bitvector is smaller, then the "extra" bits are assumed to
   * be clear (unset).
   *
   * @param rhs A bitvector to bitwise-or with.
   */
  void operator|=(basic_bitvector const& rhs) THROW_0 {
    size_type const len = min(byte_size(m_size), byte_size(rhs.m_size));
    for (size_type i = 0 ; i < len ; ++i) {
      m_ptr[i] |= rhs.m_ptr[i];
    }
  }

  /**
   * In-place bitwise-and with another bitvector.
   *
   * The bitvector is modified to contain the result of computing the
   * bitwise-and of this bitvector with another bitvector. If the
   * other bitvector is larger, then the extra bits are ignored. If
   * the other bitvector is smaller, then the "extra" bits are assumed
   * to be clear (unset).
   *
   * @param rhs A bitvector to bitwise-and with.
   */
  void operator&=(basic_bitvector const& rhs) THROW_0 {
    size_type const len = min(byte_size(m_size), byte_size(rhs.m_size));
    for (size_type i = 0 ; i < len ; ++i) {
      m_ptr[i] &= rhs.m_ptr[i];
    }
  }

  /**
   * In-place bitwise-xor with another bitvector.
   *
   * The bitvector is modified to contain the result of computing the
   * bitwise-xor of this bitvector with another bitvector. If the
   * other bitvector is larger, then the extra bits are ignored. If
   * the other bitvector is smaller, then the "extra" bits are assumed
   * to be clear (unset).
   *
   * @param rhs A bitvector to bitwise-xor with.
   */
  void operator^=(basic_bitvector const& rhs) THROW_0 {
    size_type const len = min(byte_size(m_size), byte_size(rhs.m_size));
    for (size_type i = 0 ; i < len ; ++i) {
      m_ptr[i] ^= rhs.m_ptr[i];
    }
  }

private:
  size_type  m_size; /**< The size of the vector in number of bits. */
  pointer    m_ptr;  /**< A pointer to the data bytes. */

};

/**
 * Convenience typedef for plain bitvector.
 *
 * This is the plain bitvector with no threading support and using
 * <code>unsigned char</code> as the underlying byte.
 */
typedef basic_bitvector<std::allocator<unsigned char>, No_threading> bitvector;

#endif /* BITVECTOR_H */
_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help   : https://help.launchpad.net/ListHelp

Reply via email to