Re: PRF & PLR flags max 32?

From: Daniel A. Koepke (dkoepke@circlemud.org)
Date: 01/24/02


On Sun, 20 Jan 2002, Elie Rosenblum wrote:

> I've looked at them both, and they're interesting, but they're not as
> close to drop in replacements for the current bitvector_t.

You can ignore std::vector<bool>.  It's ugly, stupid, and on the chopping
block for C++0x.  A specialization of a container that is not, itself, a
container is confusing and broken.  std::vector<bool> _looks_ like a
container, but cannot be used like one.

std::bitset<N> merits a bit more attention (no pun intended).  It can be
used as a drop-in replacement for bitvector_t if desired:

  std::bitset<8> test;
  test |= (1 << 0) | (1 << 2); // E.g., test |= 5;
  std::cout << test << std::endl;

produces:

  00000101

which is the same bit pattern as if test were an unsigned long.  That
works, but it's not really what we want to do.  You can do the same thing
as you have done with your bitvect and make all your flags std::bitset<N>
variables, but this a lot of overhead (for both approaches) for little
gain.  It'd be better just to not use IS_SET, etc., with bitwise OR'd
flags and, indeed, there aren't many places where the relevant types of
flags are used in this way in CircleMUD (relevant being: AFF_x, PLR_x,
PRF_x, MOB_x, ITEM_x, etc.; some, like MAG_x, TO_x, and FIND_x don't need
extensibility), then you could make the flags enums and change the macros:

  #define IS_SET(bitv, flag)            bitv.test(flag)
  #define SET_BIT(bitv, flag)           bitv.set(flag)
  #define REMOVE_BIT(bitv, flag)        bitv.reset(flag)
  #define TOGGLE_BIT(bitv, flag)        bitv.flip(flag)

This buys you an "infinite" number of bits, with less overhead than your
approach.

> Having looked at the code for the template, it's rather useful
> generally but I would end up gutting it anyway just to get an
> optimized version.

To be honest, the standard STL implementation of std::bitset<N> is
considerably better optimized than your bitvect class.  You're doing a lot
of unnecessary copying[1] and have a ton of space overhead.  This is not
even accounting for the one-word bit set specialization.  In addition,
system headers can often take advantage of knowing how the compiler
optimizes things or what architecture you're on, and provide more
efficient implementations than you would want to by hand (e.g., it might
implement some copies in assembly).

A good, easy start to optimizing your code would be to go through and use
const references as arguments in functions where a local copy is not
necessary.  E.g., instead of your,

  inline bitvect operator |(bitvect a, bitvect b)
  { . . . }

we do,

  inline bitvect operator |(const bitvect& a, const bitvect& b)
  { . . . }

which saves two copies (meaning, two calls to the copy constructor, which
is probably a loop over an assignment to bitvect::vector[] elements).

Your other goals, however, are not so easily deflected, and bitvect still
wins out if you're truly wishing to remain as close as possible to stock
CircleMUD.


-dak

--
   +---------------------------------------------------------------+
   | FAQ: http://qsilver.queensu.ca/~fletchra/Circle/list-faq.html |
   | Archives: http://post.queensu.ca/listserv/wwwarch/circle.html |
   | Newbie List:  http://groups.yahoo.com/group/circle-newbies/   |
   +---------------------------------------------------------------+



This archive was generated by hypermail 2b30 : 06/25/03 PDT