| Lars Gullik Bjønnes wrote: >> > >>>>istreambuf_iterator: >>>>time ./test_crc /boot/vmlinuz-2.4.9-13 >>>>CRC: 2642954630 >>>> >>>>real 0m0.252s >>>>user 0m0.130s >>>>sys 0m0.000s >>>> >>>> >>>>istream_iterator: >>>> time ./test_crc /boot/vmlinuz-2.4.9-13 >>>>CRC: 2642954630 >>>> >>>>real 0m0.550s >>>>user 0m0.260s >>>>sys 0m0.010s >>>> >>>>(box compiling LyX at the time so there was a bit of load...) >>
Lars, would you mind sending me the source of that test program, and I'll test out this patch with the two different implementations and compare results? I think the biggest issue with this patch is portability, which is also looked after by autoconf, so if I can regression test it against the previous implementation and make it all come out the same we should be OK. Lars Gullik Bjønnes wrote: > >| +// Figure out what iterator implementation we are going to use... >| +// This currently selects mmap over istreambuf_iterator. >| +#ifdef HAVE_MMAP >| + #ifdef __GNUC__ > >Hmm WITH_WARNINGS exist in 1.1.6 too. > > > >| + #warning lyx::sum() using mmap (lightning fast but untested) >| + #endif >| + #define USE_MMAP >| +#elif HAVE_DECL_ISTREAMBUF_ITERATOR >| + #ifdef __GNUC__ >| + #warning lyx::sum() using istreambuf_iterator (fast) >| + #endif >| + #define USE_ISTREAMBUF_ITER >| + #define USE_STREAM >| +#else >| + #ifdef __GNUC__ >| + #warning lyx::sum() using istream_iterator (slow as a snail) >| + #endif >| + #define USE_ISTREAM_ITER >| + #define USE_STREAM >| +#endif > >And why are these new defines needed? Can't we just use > >HAVE_MMAP, HAVE_DECL_ISTREAMBUF_ITERATOR directly? > This seemed simpler... to decide which implementation to use when you may have more than one available. Extending the original scheme would have been *really* ugly, with #if defined(this) && !defined(that) all over the place. It also simplifies the macros when you have to check multiple things eg I have now discovered I should check HAVE_MMAP and HAVE_MUNMAP, as well as possibly one other macro. With this arrangement, I only have to change it in *one* place. It also allows me to conveniently control inclusion of header files, without writing #if blah blah blah everywhere and risking not changing them all in a future mod. I'd rather keep this. > > >| + char *beg = (char*)mm; >| + char *end = ((char*)mm)+info.st_size; > >Ok, I know I used c-style cast in my example, but here we should use >C++-style casts. static_cast or reinterpret_cast here. > OK, I'm figuring out which one we should use. > >And we should find a way to avoid this much ifdef clutter. > I'm thinking of breaking it into two separate implementations, one for mmap and one for streams, controlled by #ifdefs. It's looking much tidier. Lars, thanks for taking the time to make comments. Ben.
