Can I use memcpy in C++ to copy classes that have

2019-01-13 04:54发布

Say I have a class, something like the following;

class MyClass
{
public:
  MyClass();
  int a,b,c;
  double x,y,z;
};

#define  PageSize 1000000

MyClass Array1[PageSize],Array2[PageSize];

If my class has not pointers or virtual methods, is it safe to use the following?

memcpy(Array1,Array2,PageSize*sizeof(MyClass));

The reason I ask, is that I'm dealing with very large collections of paged data, as decribed here, where performance is critical, and memcpy offers significant performance advantages over iterative assignment. I suspect it should be ok, as the 'this' pointer is an implicit parameter rather than anything stored, but are there any other hidden nasties I should be aware of?

Edit:

As per sharptooths comments, the data does not include any handles or similar reference information.

As per Paul R's comment, I've profiled the code, and avoiding the copy constructor is about 4.5 times faster in this case. Part of the reason here is that my templated array class is somewhat more complex than the simplistic example given, and calls a placement 'new' when allocating memory for types that don't allow shallow copying. This effectively means that the default constructor is called as well as the copy constructor.

Second edit

It is perhaps worth pointing out that I fully accept that use of memcpy in this way is bad practice and should be avoided in general cases. The specific case in which it is being used is as part of a high performance templated array class, which includes a parameter 'AllowShallowCopying', which will invoke memcpy rather than a copy constructor. This has big performance implications for operations such as removing an element near the start of an array, and paging data in and out of secondary storage. The better theoretical solution would be to convert the class to a simple structure, but given this involves a lot of refactoring of a large code base, avoiding it is not something I'm keen to do.

10条回答
Lonely孤独者°
2楼-- · 2019-01-13 05:14

[...] but are there any other hidden nasties I should be aware of?

Yes: your code makes certain assumptions that are neither suggested nor documented (unless you specifically document them). This is a nightmare for maintenance.

Also, your implementation is basically hacking (if it's necessary it's not a bad thing) and it may depend (not sure on this) on how your current compiler implements things.

This means that if you upgrade compiler / toolchain one year (or five) from now (or just change optimization settings in your current compiler) nobody will remember this hack (unless you make a big effort to keep it visible) and you may end up with undefined behavior on your hands, and developers cursing "whoever did this" a few years down the road.

It's not that the decision is unsound, it's that it is (or will be) unexpected to maintainers.

To minimize this (unexpectedness?) I would move the class into a structure within a namespace based on the current name of the class, with no internal functions in the structure at all. Then you are making it clear you're looking at a memory block and treating it as a memory block.

Instead of:

class MyClass
{
public:
    MyClass();
    int a,b,c;
    double x,y,z;
};

#define  PageSize 1000000

MyClass Array1[PageSize],Array2[PageSize];

memcpy(Array1,Array2,PageSize*sizeof(MyClass));

You should have:

namespace MyClass // obviously not a class, 
                  // name should be changed to something meaningfull
{
    struct Data
    {
        int a,b,c;
        double x,y,z;
    };

    static const size_t PageSize = 1000000; // use static const instead of #define


    void Copy(Data* a1, Data* a2, const size_t count)
    {
        memcpy( a1, a2, count * sizeof(Data) );
    }

    // any other operations that you'd have declared within 
    // MyClass should be put here
}

MyClass::Data Array1[MyClass::PageSize],Array2[MyClass::PageSize];
MyClass::Copy( Array1, Array2, MyClass::PageSize );

This way you:

  • make it clear that MyClass::Data is a POD structure, not a class (binary they will be the same or very close - the same if I remember correctly) but this way it is also visible to programmers reading the code.

  • centralize the usage of memcpy (if you have to change to a std::copy or something else) in two years you do it in a single point.

  • keep the usage of memcpy near the implementation of the POD structure.

查看更多
再贱就再见
3楼-- · 2019-01-13 05:14

When talking about the case you're referring to I suggest you declare struct's instead of class'es. It makes it a lot easier to read (and less debatable :) ) and the default access specifier is public.

Of course you can use memcpy in this case, but beware that adding other kinds of elements in the struct (like C++ classes) is not recommended (due to obvious reasons - you don't know how memcpy will influence them).

查看更多
▲ chillily
4楼-- · 2019-01-13 05:16

Your class has a constructor, and so is not POD in the sense that a C struct is. It is therefore not safe to copy it with memcpy(). If you want POD data, remove the constructor. If you want non-POD data, where controlled construction is essential, don't use memcpy() - you can't have both.

查看更多
Melony?
5楼-- · 2019-01-13 05:23

It will work, because a (POD-) class is the same as a struct (not completely, default access ...), in C++. And you may copy a POD struct with memcpy.

The definition of POD was no virtual functions, no constructor, deconstructor no virtual inheritance ... etc.

查看更多
登录 后发表回答