On Mon, Nov 9, 2015 at 10:24 AM, Sage Weil <sweil@xxxxxxxxxx> wrote: > On Mon, 9 Nov 2015, Gregory Farnum wrote: >> On Wed, Nov 4, 2015 at 7:07 AM, Gregory Farnum <gfarnum@xxxxxxxxxx> wrote: >> > The problem with this approach is that the encoded versions need to be >> > platform-independent ? they are shared over the wire and written to >> > disks that might get transplanted to different machines. Apart from >> > padding bytes, we also need to worry about endianness of the machine, >> > etc. *And* we often mutate structures across versions in order to add >> > new abilities, relying on the encode-decode process to deal with any >> > changes to the system. How could we deal with that if just dumping the >> > raw memory? >> > >> > Now, maybe we could make these changes on some carefully-selected >> > structs, I'm not sure. But we'd need a way to pick them out, guarantee >> > that we aren't breaking interoperability concerns, etc; and it would >> > need to be something we can maintain as a group going forward. I'm not >> > sure how to satisfy those constraints without burning a little extra >> > CPU. :/ >> > -Greg >> >> So it turns out we've actually had issues with this. Sage merged >> (wrote?) some little-endian-only optimizations to the cephx code that >> broke big-endian systems by doing a direct memcpy. Apparently our >> tests don't find these issues, which makes me even more nervous about >> taking that sort of optimization into the tree. :( > > I think the way to make this maintainable will be to > > 1) Find a clean approach with a simple #if or #ifdef condition for > little endian and/or architectures that can handle unaligned int pointer > access. > In C++ you can also do that with a template <bool Endian> or using std::enable_if. The upside is the same as the downside (depending on how you look at it). So it'll add to compile time checks (because it won't be discarded by the processor) and it'll take longer to build, but you get extra checks and the compiler will later discard the unused code. If you do that, it should be easier to write unit tests for the functionality. > 2) Maintain the parallel optimized implementation next to the generic > encode/decode in a way that makes it as easy as possible to make changes > and keep them in sync. > > 3) Optimize *only* the most recent encoding to minimize complexity. > > 4) Ensure that there is a set of encode/decode tests that verify they both > work, triggered by make check (so that a simple make check on a big > endian box will catch errors). Ideally this'd be part of the > test/encoding/readable.sh so that we run it over the entire corpus of old > encodings.. > > > sage > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Milosz Tanski CTO 16 East 34th Street, 15th floor New York, NY 10016 p: 646-253-9055 e: milosz@xxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
