blitz icon indicating copy to clipboard operation
blitz copied to clipboard

Faster fastiter!

Open EddieBreeveld opened this issue 6 years ago • 10 comments
trafficstars

We have made some minor modifications to the code in fastiter.h code. Currently operator() returns a copy of the data at element i (in our version of Blitz++ it returns a T_numtype). We found that by returning a const reference instead (const T_numtype&) we can get much faster array element handling with complex types (e.g. arrays of CStrings). We made changes in 6 places in the code in this file, so that all T_numtype return values are replaced with const references.

In one of our models this change resulted in a 25% improvement in run-time, without changing the results in any of our regression tests.

The latest version of Blitz++ returns a T_result instead of a T_numtype, with the comment "if T_numtype is POD, then T_result is T_numtype, but if T_numtype is an ET class, T_result will be the array class for that class." Unfortunately a CString is neither plain ordinary data nor an expression template class, and copying CStrings is expensive.

I hope this notice will allow us to distribute the modified version of Blitz++ to our clients.

All the best, Eddie

EddieBreeveld avatar Feb 21 '19 08:02 EddieBreeveld

You can distribute whatever you like, no notice required. But a pill request on github would be nice so others can benefit from your improvements as well. And so you don’t have to maintain it in the future.

Thank you, Elizabeth

On Thu, Feb 21, 2019 at 03:43 Edward Breeveld [email protected] wrote:

We have made some minor modifications to the code in fastiter.h code. Currently operator() returns a copy of the data at element i (in our version of Blitz++ it returns a T_numtype). We found that by returning a const reference instead (const T_numtype&) we can get much faster array element handling with complex types (e.g. arrays of CStrings). We made changes in 6 places in the code in this file, so that all T_numtype return values are replaced with const references.

In one of our models this change resulted in a 25% improvement in run-time, without changing the results in any of our regression tests.

The latest version of Blitz++ returns a T_result instead of a T_numtype, with the comment "if T_numtype is POD, then T_result is T_numtype, but if T_numtype is an ET class, T_result will be the array class for that class." Unfortunately a CString is neither plain ordinary data nor an expression template class, and copying CStrings is expensive.

I hope this notice will allow us to distribute the modified version of Blitz++ to our clients.

All the best, Eddie

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/98, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1cd2fO_1AJoD0EKIe7rMyIrbKFFUR5ks5vPlwpgaJpZM4bG7KG .

citibeth avatar Feb 21 '19 11:02 citibeth

+1 for a PR, Thanks in advance

slayoo avatar Feb 21 '19 14:02 slayoo

I've not done if before, and we are using a very old version of Blitz++, but I will try a 'pull request'. I will need to read up what 'pull request' means first :) (We use Azure DevOps - which used to be called Visual Studio Team Services, which was Microsoft Team Foundation Server)

EddieBreeveld avatar Feb 21 '19 15:02 EddieBreeveld

I have concerns about such a change, because I suspect that returning references will inhibit automatic vectorization on the platforms that support this. If you're operating on strings, your operations are probably going to be slow in any case, so I'm not sure Blitz is a good choice for such operations. What's the use case for evaluating expressions of strings?

Cheers,

/Patrik

On Wed, Feb 20, 2019 at 10:43 PM Edward Breeveld [email protected] wrote:

We have made some minor modifications to the code in fastiter.h code. Currently operator() returns a copy of the data at element i (in our version of Blitz++ it returns a T_numtype). We found that by returning a const reference instead (const T_numtype&) we can get much faster array element handling with complex types (e.g. arrays of CStrings). We made changes in 6 places in the code in this file, so that all T_numtype return values are replaced with const references.

In one of our models this change resulted in a 25% improvement in run-time, without changing the results in any of our regression tests.

The latest version of Blitz++ returns a T_result instead of a T_numtype, with the comment "if T_numtype is POD, then T_result is T_numtype, but if T_numtype is an ET class, T_result will be the array class for that class." Unfortunately a CString is neither plain ordinary data nor an expression template class, and copying CStrings is expensive.

I hope this notice will allow us to distribute the modified version of Blitz++ to our clients.

All the best, Eddie

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/98, or mute the thread https://github.com/notifications/unsubscribe-auth/AFK8GCPR-uWq_RbdQiVoy5kbe-Fb9TTBks5vPlwpgaJpZM4bG7KG .

lutorm avatar Feb 22 '19 01:02 lutorm

Hi Patrik, Many thanks for your comment. We will carefully evaluate our test models before and after the proposed changes. With the VS2017 Microsoft compiler the number of automatically vectorised loops in one of our more complex models has not changed (51). This may not be the case for everybody however.

Our product is a code generator for Actuaries. Within the product our clients can choose variables with four datatypes: Date (int underneath), Indicator (also an int), Numeric (double) and Character (CString). With any of these they can create arrays of up to five dimensions, and manipulate them in many ways. We do discourage the use of Character arrays, but this is sometimes ignored. These multi dimensional arrays can be indexed by policy number, date, interest rate, etc, etc., in fact by anything that the client chooses, and manipulated by many functions. When converting from FORTRAN more than 15 years ago we found that Blitz++ was the only C++ product that could be used to help us meet this requirement, and it was very fast too.

All the best, Eddie

EddieBreeveld avatar Feb 22 '19 08:02 EddieBreeveld

On Thu, Feb 21, 2019 at 8:19 PM Patrik Jonsson [email protected] wrote:

I have concerns about such a change, because I suspect that returning references will inhibit automatic vectorization on the platforms that support this.

I am doubtful that this is the case. Everything else in C++ (operator[] for example) returns a reference, and compilers manage. In the absence of evidence to the contrary, I believe we should assume that returning a reference instead of a value will have no performance impact on vectorization. Or in this case, a performance improvement. That makes sense; because the compiler has more options on what to do when a reference is returned, vs. a value.

If you're operating on strings, your operations are probably going to be slow in any case, so I'm not sure Blitz is a good choice for such operations. What's the use case for evaluating expressions of strings?

Multi-dimensional arrays, and the ability to iterate through them, are a fundamental language feature that is missing in C++, but that Blitz provides. They are commonly used in scientific programming for double and (less frequently) int; but they should also be available for any other type.

citibeth avatar Feb 22 '19 13:02 citibeth

All I know is that when I coded up the vectorization improvements a long time ago, there were numerous things that "should" not matter that did. The blitz ETs tax the optimizers to the max and something they may understand in a simple statement did not necessarily work when they had to dig through all the ET machinery in a complicated blitz expression. It's possible things have changed and vectorization is better and more robust now, but I would urge you not to assume that it won't matter. Most of the little details in the ET machinery is (or at least was) there for a reason.

On Fri, Feb 22, 2019 at 3:57 AM Elizabeth Fischer [email protected] wrote:

On Thu, Feb 21, 2019 at 8:19 PM Patrik Jonsson [email protected] wrote:

I have concerns about such a change, because I suspect that returning references will inhibit automatic vectorization on the platforms that support this.

I am doubtful that this is the case. Everything else in C++ (operator[] for example) returns a reference, and compilers manage. In the absence of evidence to the contrary, I believe we should assume that returning a reference instead of a value will have no performance impact on vectorization. Or in this case, a performance improvement. That makes sense; because the compiler has more options on what to do when a reference is returned, vs. a value.

If you're operating on strings, your operations are probably going to be slow in any case, so I'm not sure Blitz is a good choice for such operations. What's the use case for evaluating expressions of strings?

Multi-dimensional arrays, and the ability to iterate through them, are a fundamental language feature that is missing in C++, but that Blitz provides. They are commonly used in scientific programming for double and (less frequently) int; but they should also be available for any other type.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/issues/98#issuecomment-466404487, or mute the thread https://github.com/notifications/unsubscribe-auth/AFK8GJpP2BbhferxMTEWIEb7oU2HGlsMks5vP_dGgaJpZM4bG7KG .

lutorm avatar Feb 22 '19 18:02 lutorm

Patrik and Elizabeth, Thanks for your useful comments. I am using an old version of Blitz++, and the fastiter changes make a good improvement in that version with the latest Microsoft compiler and Intel processors. I have looked into but unfortunately don't have the time to learn about GitHub branching and 'pull requests' at the moment. We will use the fastiter changed version for our clients, but until I am able to use the latest Blitz++ and verify my changes there, I will not do any more. (I hope I have described the changes sufficiently if someone else want to try them). All the best, Eddie

EddieBreeveld avatar Feb 25 '19 08:02 EddieBreeveld

@EddieBreeveld would it be possible to share either: a) Some diffs of your changes b) The source files you changed, along with the tarball of the version of Blitz++ you downloaded before you started making changes?

Either of these would allow someone else to re-create your changes as a pull request.

citibeth avatar Mar 03 '19 23:03 citibeth

We use Blitz++ from about 2007. A few years ago I tried a later version and found it ran slower, so we stayed with the older version. Attached are the before and after versions of the fastiter.h file. Please ask if you want a zip of the whole distribution. Desktop.zip All the best, Eddie

EddieBreeveld avatar Mar 04 '19 11:03 EddieBreeveld