My Authors
Read all threads
I thought I’d do some real code review for the @Imperial_JIDEA CovIdSim, rather than the nonsense political one published. Not exhaustive, but hopeful constructive. /1
So the first thing I noticed when trying to get it going is that scanning the population file takes a long time: 77.96 seconds in this run. /2
And further research showed that just measuring the file was taking 14.4 seconds. It’s a big file, but that’s longer than it should be, so what is going wrong? /3
So this simple and easy to understand code is causing the system to read a buffer, copy into another buffer, and they are then discarding it. I think I’ve written code like this before, but not to read files as big as this one. /4
First thing is first then is to change that code to read the file and determine the line ends. I’ve included some folding just to tip it down to my target time. /5
Now the measuring code is being done in about half a second. If I’m working it out right, that’s a speed improvement of 96.5%. /6
The second part of involved number conversion of this huge file, so it wasn’t going to see that kind of speed up, but there were gains to be had. For example, reducing the double buffering. /7
If the standard library isn’t caching the pointer, then the computer is reparsing this format string each time, and yet the order doesn’t change. /8
There aren’t many more things more costly in code than a division, and one in an ‘if’ statement that does nothing is really very costly. /9
Finally they are reading into variables and then copying into a struct. Maybe someone was uncomfortable passing struct members and thought this was safer memory-wise? Otherwise I can't think why this was done this way. /10
This time the solution is more involved. We need to break the chunks we take in memory into valid entire lines to be parsed. /11
Once that has been done, we can read each parameter in a tight loop. /12
At the very end I noticed I could reduce some maths by dropping out of the loop early, but didn’t notice that this mean the calculation for the next line could have been optimised too, but the gain would have been insignificant and I left it. /13
And that brings us within the 30 second barrier to read in a 489 megabyte file of numbers.

(The measuring code I wrote in less than 10 minutes, but I can’t claim that of the file reading bit.) /14
And here is the longest 28.4 seconds of your life, but not as long as the 76 seconds it was. A 63% improvement (I think). /15
OK, I also found a bug. In the documentation it says the population file is optional, but without it P.NC is -1, and ‘i’ can tell you that nothing good comes from the square root of -1. /16
Yes, we’re doing maths jokes, and also not being a maths genius, I feel I should point out that this works out the square root and then squares it. Could be a maintenance thing, a future proof thing, or could be deliberate. /17
Finally, the documentation says it was written in C++, and I don't think this is accurate. /18
There are some dead giveaways that this was not written in C++. For starters the constant files consist of #DEFINE declarations with actual constants being rarely every employed as constants. /19
This is an indicator of C coding, because constants weren’t always in C like they were in C++, and when they were introduced there was an overhead, which meant not all engineers switched to them. /20
Another tell-tale sign this was written in C is the large number of variable lists that can be found at the top of functions. /21
Again, this is partly for historic reasons and partly because in C++ putting all your variables at the top could result in code being run for objects that were never used. /22
Finally, and the biggest clue comes from the fact they are using typedef to declare their structs in a way that allows them to be referred to directly by type. /23
C++ made this redundant, and that leaves us with very clear distinctions on how data structures are declared, allocated and deleted. /24
The code adopts the C form throughout the code and I have seen no exception to this, and although I didn’t look through every line of code in every file, I didn’t see anything that represented something unique to C++. /25
I started programming in C++ when C was still big, and it’s easy to recognise the code of one or the other and understand what they are and why they do things differently. This was written in C or by someone who thought they were programming in C++ and didn't know it wasn't. /26
Ignoring my language pedantry, the main takeaway of the exercise should be that things like parallelisation should not be relied on to speed up initialisation. /27
Because I’ve just sped the initialisation up by nearly 48 seconds, not by employing parallelisation, or by offloading to the GPU, but by making some slow code more efficient. 28/28
Missing some Tweet in this thread? You can try to force a refresh.

Enjoying this thread?

Keep Current with Steve Rawlings

Profile picture

Stay in touch and get notified when new unrolls are available from this author!

Read all threads

This Thread may be Removed Anytime!

Twitter may remove this content at anytime, convert it as a PDF, save and print for later use!

Try unrolling a thread yourself!

how to unroll video

1) Follow Thread Reader App on Twitter so you can easily mention us!

2) Go to a Twitter thread (series of Tweets by the same owner) and mention us with a keyword "unroll" @threadreaderapp unroll

You can practice here first or read more on our help page!

Follow Us on Twitter!

Did Thread Reader help you today?

Support us! We are indie developers!


This site is made by just two indie developers on a laptop doing marketing, support and development! Read more about the story.

Become a Premium Member ($3.00/month or $30.00/year) and get exclusive features!

Become Premium

Too expensive? Make a small donation by buying us coffee ($5) or help with server cost ($10)

Donate via Paypal Become our Patreon

Thank you for your support!