On Sun, 06 Mar 2011 23:50:55 +0100 Ivan Le Lann <ivan.lelann@free.fr> wrote:
Le dimanche 06 mars 2011 à 08:20 -0500, Chad Nelson a écrit :
1. Parametrize with container type. [...]
I actually tested earlier iterations with containers such as vector for their storage. The performance numbers were noticeably worse.
You're restricting this to std::vector against your array, when many container could fit.
std::vector is the most logical STL container, given the code's needs. Though std::deque might be almost as good, perhaps better in some cases because it doesn't require large blocks of contiguous memory to store massive numbers.
I did following modifications on reviewed XInt:
- naively turn magnitude_manager_t into a boost::array wrapper - add byref args in integer, raw_integer (and others, to compile) - nothing else
(NB: I used the original with byref too as this was faster)
I'm nearly finished converting most arguments to by-reference myself, that alone should improve the speed some.
I used dummy code introduced by Phil Endecott with various fixed lengths. Even at 20000 bits, boost::array version is almost twice faster. At 2000 bits, thrice faster. I did not optimize boost::array usage, using plain zero init in constructor, and I probably missed some copies. Oh, by the way, boost::array version should be threadsafe.
I'd have to see the actual changes you've made, but I'd be interested to see what difference that same change will make once I've finished the current modifications. For fixed-length integers, it might be a cheap and useful speed improvement.
My point: You're making choices in place of your user. This is a dangerous way. If you like container genericity idea suggested by the reviewer, I might try to provide my humble and limited help. I do not consider this exposing internals, but rather keeping the internals out. There is too much code in your library, actually.
I do like the idea, just not quite yet. :-)
Having said that, your library works well and seems easy to hack. A very good point for the future. I'm not experienced enough to properly review it but that's definitely a good one IMHO.
Thanks, and thanks for your input. -- Chad Nelson Oak Circle Software, Inc. * * *