3
\$\begingroup\$

My code is not very elegant, but I'm a total newbie in c++ and I consider it already a major achievement that I managed to write something that actually works.

As background, I'm defining in VBA the function below, contained in "FieldTrans.dll":

Private Declare Function ShannonInterp Lib "[Here the name of my path]\FieldTrans.dll" _
(ByVal Matrix As Variant, ByVal MatrixHeight As Integer, ByVal MatrixWidth As Integer, ByVal Res As Double, ByVal DeltaTheta As Double, ByVal DeltaPhi As Double) As Variant

and using it by having:

Dim InterpCo As Variant
InterpCo = ShannonInterp(AmplCo, PointsTheta, PointsPhi, Resolution, 181 / (PointsTheta+1), 361 / (PointsPhi+1))

I thought it'd work like a charm, but it doesn't (yet). Meanwhile I'd like more improvement on the execution time. Therefore the only thing I can optimize is the c++ code contained in the dll, which is:

// FieldTrans.h
#ifdef FIELDTRANS_EXPORTS
#define FIELDTRANS_API __declspec(dllexport)
#else
#define FIELDTRANS_API __declspec(dllimport)
#endif
#include <oaidl.h>

    // Returns the normalized cardinal sinus coefficient
    FIELDTRANS_API double _stdcall SinC(double x);
extern "C"
{
    // Returns the interpolated field matrix: 180° x 360° with Res step
    FIELDTRANS_API VARIANT _stdcall ShannonInterp(VARIANT Matrix, int MatrixHeight, int MatrixWidth, double Res, double DeltaTheta, double DeltaPhi);
};


// FieldTrans.cpp
#include "stdafx.h"
#include "FieldTrans.h"
#include <math.h>
#include <vector>
#include <oaidl.h>

double _stdcall SinC(double x)
{
    static const double PI = 3.14159265358979323846;
    double y;
    y = 1;
    if (x != 0) y = sin(PI * x) / (PI * x);
    return y;
}

VARIANT _stdcall ShannonInterp(VARIANT Matrix, int MatrixHeight, int MatrixWidth, double Res, double DeltaTheta, double DeltaPhi)
{
    int k, l, m, n;
    double iRes, jRes;
    double ThetaSinC, PhiSinC;
    int Height = static_cast<int> (180 / Res + 1);
    int Width = static_cast<int> (360 / Res + 1);
    k = 0; m = 1;
    long indi[2];
    VARIANT A, Interpolated;
    Interpolated.vt = VT_ARRAY | VT_VARIANT;
    SAFEARRAYBOUND sab[2];
    sab[0].lLbound = 0; sab[0].cElements = Height;
    sab[1].lLbound = 0; sab[1].cElements = Width;
    Interpolated.parray = SafeArrayCreate(VT_VARIANT, 2, sab);
    for (int i = 0; i < Height; i++)
    {
        iRes = i * Res;
        do { k = k + 1; m = k + 1; } while (iRes > (k + 1) * DeltaTheta);
        if (m > MatrixHeight) m = MatrixHeight - 1;
        ThetaSinC = SinC((iRes - k * DeltaTheta) / DeltaTheta);
        l = 0; n = 1;
        for (int j = 0; j < Width; j++)
        {
            jRes = j * Res;
            do { l = l + 1; n = l + 1; } while (jRes > (l + 1) * DeltaPhi);
            if (n > MatrixWidth) n = 0;
            PhiSinC = SinC((jRes - l * DeltaPhi) / DeltaPhi);
            indi[0] = k; indi[1] = l;   // Selects Matrix[k][l]
            SafeArrayGetElement(Matrix.parray, indi, &A);
            A.dblVal += A.dblVal * ThetaSinC * PhiSinC;
            indi[0] = m; indi[1] = l;   // Selects Matrix[m][l]
            SafeArrayGetElement(Matrix.parray, indi, &A);
            A.dblVal += A.dblVal * (1 - ThetaSinC) * PhiSinC;
            indi[0] = k; indi[1] = n;   // Selects Matrix[k][n]
            SafeArrayGetElement(Matrix.parray, indi, &A);
            A.dblVal += A.dblVal *  ThetaSinC * (1 - PhiSinC);
            indi[0] = m; indi[1] = n;   // Selects Matrix[m][n]
            SafeArrayGetElement(Matrix.parray, indi, &A);
            A.dblVal += A.dblVal *  (1 - ThetaSinC) * (1 - PhiSinC);
            indi[0] = i; indi[1] = j;   // Puts the result of the calculation into Interpolated[i][j]
            SafeArrayPutElement(Interpolated.parray, indi, &A); 
        }
    }
    return Interpolated;
}

Would you happen to have any advice for me? (please have mercy I am REALLY a dummy!)

P.S.: Notice that I "wrapped" a lot of code in single lines to save some time to the processor going through them... I know it's not conventional, but I'm really looking to speed this up the most...

\$\endgroup\$
2
\$\begingroup\$

Formatting

Notice that I "wrapped" a lot of code in single lines to save some time to the processor going through them... I know it's not conventional, but I'm really looking to speed this up the most...

Let's start with that. At least in every implementation of which I'm aware, C++ is compiled. The compiler reads through your source code, converts it to machine code, links in any libraries you've used, and generates an executable file as its output. The speed of execution is completely unrelated to the source code formatting. Moving more onto one line will not make a single iota of difference in the speed at which the code executes.

Speed

Since you say speed is a major concern, the next obvious point would probably be the use of a "safe array". Even if your result needs to go in a safe array, you probably want to use something like std::vector during the computation, then copy the result into a safe array when you're done. At least at first glance, it looks like your algorithm is \$O(N^2)\$, so for a large matrix, this could still be a pretty big win--at least the last time I looked, SafeArrayGetElement and SafeArrayPutElement were quite a bit slower than vector's operator[].

\$\endgroup\$
1
  • \$\begingroup\$ Thanks, this was very helpful to write my code in a better, more understandable fashion. \$\endgroup\$ May 3 '17 at 6:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.