Improve performance of MockExpectedCallsList#778
Improve performance of MockExpectedCallsList#778mmartalo wants to merge 4 commits intocpputest:masterfrom
Conversation
For large number (e.g. 10000) of mocked calls, the simple linked list was a performance bottleneck. This patch brings two improvements: - tracking the last element of the linked list (for O(1) insertion) - deleting NULL elements as they are marked, as opposed to rescanning the list from scratch after marking. Profiling with callgrind shows that the linked list structure is no longer the performance bottleneck.
There was a problem hiding this comment.
I think keeping the iteration algorithm in one place is good, but I'm ok with inlining this everywhere if you find it ugly.
PREV should be changed to PARENT
|
Great! Do you suppose you could do the same without using Std C Lib assert()? The thing is that CppUTest shouldn't depend on the Std C library, and the inclusion of assert.h breaks that. This is why you get a failure on Travis CI - it's a run specifically designed to check for compilation without Std. C library. |
Asserts left in the code as comments
|
Oh ok, I didn't know that. I was wondering how assert.h could be missing :) |
|
There you go. About the comments / asserts, let's see what @basvodde has to say. Looking forward to this being merged.... |
|
Any news on this? If the PR needs changes, please let me know this week, as I'll be unavailable for a couple of weeks afterwards. |
|
Sorry! I won't have time to merge it this week, but I'll definitively will request some small changes. That said, I'm pretty sure eventually it'll be merged as this was one problem that needed to be solved. So thanks for that and sorry for the slow response during these days. |
3 participants
For large number (e.g. 10000) of mocked calls, the simple linked list
was a performance bottleneck.
This patch brings two improvements:
the list from scratch after marking.
Profiling with callgrind shows that the linked list structure is no
longer the performance bottleneck.
I know that having a huge number of mocked/expected calls is debatable, but it did happen to matter in my use case. Basically, I'm doing checks of the type: perform an operation, and assert that a certain log message is displayed. This means that MockExpectedCallsList has to scale to the number of logs printed by the operation.
As you see from the Callgrind results for this particular test case, addExpectedCall was spending most of its time in self(). Where the vast majority of this time went was into looping through the linked list in order to find the tail, so that it can insert the new element.
Profiling overview before patch:
Profiling before patch, zoom in on addExpectedCall:
Profiling overview after patch: