简单矩阵类-版本2

Question

正如在我的前一个问题上所建议的那样，我正在发布我修改过的代码以获得反馈。

有一件事引起了我的注意，那就是我的程序在添加修改后运行得更慢。以前，它在一台单核机器上每秒运行128次迭代，在4核计算机上运行382次。它现在运行在125在单一核心和360在四核机器.我不确定我在第一个版本中管理内存的方式是否更快，或者我是否实现错了。

Grid3d.h：

#ifndef _GRID3D_
#define _GRID3D_

#include <iostream>

class Grid3d
{
private:
    size_t M, N, L;
    double* buffer;
    size_t compute_index(size_t, size_t, size_t) const;

public:
    //Constructores
    Grid3d(size_t,size_t,size_t);
    Grid3d();
    Grid3d(const Grid3d&);
    ~Grid3d();

    //Operadores
    double& operator()(size_t, size_t, size_t);
    double  operator()(size_t, size_t, size_t) const;
    Grid3d& operator=(Grid3d);

    //Acceso
    int get_L();
    int get_M();
    int get_N();
    double get(size_t,size_t,size_t) const;
    void reset();
};

std::ostream & operator<< (std::ostream &,Grid3d);
#endif

Grid3d.cc：

#include "Grid3d.h"
#include <cmath>
#include <iomanip>
#include <cassert>

using namespace std;

//------------------------------------------------------------------
// Constructores 
//------------------------------------------------------------------

//Constructor
Grid3d::Grid3d(size_t M, size_t N, size_t L)
      : M(M), N(N), L(L), buffer(new double[M * N * L])
{
    for (size_t l=0;l<M*N*L;l++){
        buffer[l] = NAN;
    }
}

//Constructor vacío
Grid3d::Grid3d()
    :M(0), N(0), L(0)
{
    buffer = NULL;
}


//Constructor copia
Grid3d::Grid3d(const Grid3d &other)
    :M(other.M), N(other.N), L(other.L)
{
    buffer = new double[M * N * L];
    for (size_t l=0;l<M*N*L;l++){
        buffer[l] = other.buffer[l];
    }
}

//Destructor
Grid3d::~Grid3d() { delete [] buffer; }

//------------------------------------------------------------------
// Operadores 
//------------------------------------------------------------------

//Access operator ():
double& Grid3d::operator()(size_t i, size_t j, size_t k)
{
    assert(i >= 0 and i < M);
    assert(j >= 0 and j < N);
    assert(k >= 0 and k < L);
    return buffer[compute_index(i, j, k)];
}

//Access operator () const:
double Grid3d::operator()(size_t i, size_t j, size_t k) const
{
    assert(i >= 0 and i < M);
    assert(j >= 0 and j < N);
    assert(k >= 0 and k < L);
    return buffer[compute_index(i, j, k)];
}

//Assignment operator
Grid3d& Grid3d::operator=(Grid3d other)
{
    using std::swap;
    swap(M, other.M);
    swap(N, other.N);
    swap(L, other.L);
    swap(buffer, other.buffer);
    return *this;
}

//------------------------------------------------------------------
// Acceso 
//------------------------------------------------------------------
size_t Grid3d::compute_index(size_t i, size_t j, size_t k) const
{
    return k * (M * N) + i * N + j;
}

int Grid3d::get_L()
{
    return L;
}

int Grid3d::get_M()
{
    return M;
}

int Grid3d::get_N()
{
    return N;
}

double Grid3d::get(size_t i, size_t j, size_t k) const
{
    assert(i >= 0 and i < M);
    assert(j >= 0 and j < N);
    assert(k >= 0 and k < L);
    return buffer[compute_index(i, j, k)];
}

void Grid3d::reset()
{
    for (size_t l=0;l<M*N*L;l++){
        buffer[l] = NAN;
    }
}

//------------------------------------------------------------------
// Others
//------------------------------------------------------------------

//cout
std::ostream & operator << (std::ostream & os , Grid3d g)
{
    int M = g.get_M();
    int N = g.get_N();
    int L = g.get_L();

    for (int k=0;k<L;k++){
        cout << "k = " << k << endl;
        for (int i=0;i<M;i++){
            for (int j=0;j<N;j++){
                cout.width(10);
                os << setprecision(3) << g.get(i,j,k)<< " ";
            }
            os << endl;
        }   
        os << endl;
    }
    return os;
}

Answer 1

当我有更多的时间时，我会进一步讨论这个问题，但是有几件事情突然出现了：

get_M/get_L/get_N应该返回一个std::size_t，而不是一个int。

get_M/get_L/get_N应该是const。

在你的研究领域，这也许是有意义的，但对我来说，NAN对于最初的价值来说是很奇怪的。我希望初始值不是未定义的，就是0。不过，对你最有用的东西。毕竟，初始化成NAN将提供一些方便的属性。

头中的所有size_t都应该是std::size_t。

构造函数应该使用reset，而不是复制完全相同的代码。

即使在实现文件中，也要小心使用using namespace std;。如果您想阅读的内容比以往任何时候都多，请参阅这。它的"tl;dr“是，一个潜在的应用程序崩溃的错误可能会在几年后发生对十几个文件所做的更改，但所有这些都可以通过增加5个字符(std::)来避免。

当输出潜在的大量数据时，尽量不要刷新，直到需要或结束为止(在单线程应用程序中，这几乎总是结束)。

特别是，std::endl并不等同于"\n"。std::cout << "\n";向std::couts缓冲区注入了一条新的线路。std::cout << std::endl;将换行符插入std::cout的S缓冲区，然后刷新缓冲区。这意味着您的operator<<没有真正使用缓冲IO，因为它会在每次输出后刷新。

简单修复:使用"\n“，然后在末尾执行std::flush(std::cout); (或std::cout << std::flush)。

cout.width(10);在您的operator<<中应该是os.width(10)。

Answer 2

导致性能下降的原因很可能是因为在您的上一个版本中，operator<<采用了一个Grid3d&，而在这个版本中则使用了一个Grid3d。您的operator<<签名应该是

std::ostream& operator<<(std::ostream&, const Grid3d&);

作为另一个代码改进，您应该更改复制构造函数：

Grid3d::Grid3d(const Grid3d &other)
  : L(other.L), M(other.M), N(other.N)
{
    buffer = new double[L * M * N];
    std::memcpy(buffer, other.buffer, M * N * L * sizeof(double));
}

memcpy可能比您编写的for循环提供更好的性能。

Answer 3

因此，您不必在这里处理创建为零大小的数组的NULL (并使用初始化程序列表)：

//Constructor vacío
Grid3d::Grid3d()
    :M(0), N(0), L(0)
{
    buffer = NULL;
}

// Prefer
Grid3d::Grid3d()
    :M(0), N(0), L(0), buffer(new double[N*M*L])
{}

通过像这样创建数组，剩下的代码不需要为空缓冲区设置特例。代码看起来是一样的。

现在，这看起来非常类似于其他构造函数。

你可以把这两者结合起来。

更喜欢使用初始化工具liest (即使在复制构造函数中)

Grid3d::Grid3d(const Grid3d &other)
    :M(other.M), N(other.N), L(other.L)
    ,buffer(new double[M * N * L])
{
    for (size_t l=0;l<M*N*L;l++){
        buffer[l] = other.buffer[l];
    }
}

您的访问操作员的const版本。就叫它非const版本吧。(我们知道，这两个版本实际上都没有对对象进行变异，因此，抛开不变是没有坏处的)。const版本返回一个值，因此它不公开任何数据以进行修改：

double Grid3d::operator()(size_t i, size_t j, size_t k) const
{
    return const_cast<Grid3d>(*this)(i,j,k);
}

以下各点应为：

int Grid3d::get_L()
int Grid3d::get_M()
int Grid3d::get_N()

可以时通过const引用来防止昂贵的副本。

std::ostream & operator << (std::ostream & os , Grid3d const& g)
                                               //      ^^^^^^

不喜欢使用\n而不是std::endl。它没有很大的好处，而且可以大大降低代码的速度。如果您希望首先保证整个结构都是同花顺打印，那么使用最后一个'\n‘的std::endl安装。