SumatraPDF is a medium size (120k+ loc, not counting dependencies) Windows GUI (win32) C++ code base started by me and mostly written by 2 people.
The goals of SumatraPDF are to be:
- fast
- small
- packed with features
- and yet with thoughtfully minimal UI
It’s not just a matter of pride in craftsmanship of writing code. I believe being fast and small are a big reason for SumatraPDF’s success. People notice when an app starts in an instant because that’s sadly not the norm in modern software.
The engineering goals of SumatraPDF are:
- reliable (no crashes)
- fast compilation to enable fast iteration
SumatraPDF has been successful achieving those objectives so I’m writing up my C++ implementation decisions.
I know those decisions are controversial. Maybe not Terry Davis level of controversial but still.
You probably won’t adopt them. Even if you wanted to, you probably couldn’t. There’s no way code like this would pass Google review. Not because it’s bad but becaues it’s different.
Diverging from mainstream this much is only feasible if you have total control: it’s your company or your own open-source project.
If my ideas were just like everyone else’s ideas, there would be little point in writing about them, would it?
Use UTF8 strings internally
My app only runs on Windows and a string native to Windows is WCHAR* where each character consumes 2 bytes.
Despite that I mostly use char* assumed to be utf8-encoded.
I only decided on that after lots of code was written so it was a refactoring oddysey that is still ongoing.
My initial impetus was to be able to compile non-GUI parts under Linux and Mac. I abandoned that goal but I think that’s a good idea anyway.
WCHAR* strings are 2x larger than char*. That’s more memory used which also makes the app slower.
Binaries are bigger if string constants are WCHAR*.
The implementation rule is simple: I only convert to WCHAR* when calling Windows API. When Windows API returns WCHA* I convert it to utf-8.
No exceptions
Do you want to hear a joke? “Zero-cost exceptions”.
Throwing and catching exceptions generate bloated code.
Exceptions are a non-local control flow that makes it hard to reason about program. Every memory allocation becomes a potential leak.
But RAII, you protest. RAII is a “solution” to a problem created by exceptions. How about I don’t create the problem in the first place.
Hard core #include discipline
My objects are not shy
I don’t bother with private and protected. struct is just class with guts exposed by default, so I use that.
While intellectually I understand the reasoning behind hiding implementation details in practices it becomes busy work of typing noise and then even more typing when you change your mind about visibility.
I’m the only person working on the code so I don’t need to force those of lesser intellect to write the code properly.
My objects are shy
At the same time I minimize what goes into a class, especially methods.
The smaller the class, the faster the build.
A common problem is adding too many methods to a class.
You have a StrVec class for array of strings. A lesser programmer is tempted to add Join(const char* sep) method to StrVec. A wise programmer makes it a stand-alone function: Join(const StrVec& v, const char* sep).
This is enabled by making everything in a class public. If you limit visibility you then have to use friendto allow Join() function access what it needs. Another example of “solution” to self-inflicted problems.
Minimize #ifdef
#ifdef is problematic because it creates code paths that I don’t always build.
I provide arm64, intel 32-bit and 64-bit builds but typically only develop with 64-bit intel build. Every #ifdef that branches on architecture introduces potential for compilation error which I’ll only know about when my daily ci build fails.
Consider 2 possible implementations of IsProcess64Bit():
Bad:
bool IsProcess64Bit() {
#ifdef _WIN64
return true;
#else
return false;
#endif
}
Good:
bool IsProcess64Bit() {
return sizeof(uintptr_t) == 8;
}
The bad version has a bug: it was correct when I was only doing intel builds but became buggy when I added arm64 builds.
This conflicts with the goal of smallest possible size but it’s worth it.
Stress testing
SumatraPDF supports a lot of very complex document and image formats.
Complex format require complex code that is likely to have bugs.
I also have lots of files in those formats.
I’ve added stress testing functionality where I point SumatraPDF to a folder with files and tell it to render all of them.
For greater coverage, I also simulate some of the possible UI actions users can take like searching, switching view modes etc.
Crash reporting
Heavy use of CrashIf()
C/C++ programmers are familiar with assert() macro.
CrashIf() is my version of that, tailored to my needs.
The purpose of assert / CrashIf is to add checks to detect incorrect use of APIs or invalid states in the program.
For example, if the code tries to access an element of an array at an invalid index (negative or larger than size of the array), it indicates a bug in the program.
I want to be notified about such bugs both when I test SumatraPDF and when it runs on user’s computers.
As the name implies, it’ll crash (by de-referencing null pointer) and therefore generate a crash report.
It’s enabled in debug and pre-release builds but not in release builds. Release builds have many, many users so I worry about too many crash reports.
premake to generate Visual Studio solution
Visual Studio uses XML files as a list of files in the project and build format. The format is impossible to work with in a text editor so you have no choice but to use Visual Studio to edit the project / solution.
To add a new file: find the right UI element, click here, click there, pick a file using file picker, click again.
To change a compilation setting of a project or a file? Find the right UI element, click here, click there, type this, confirm that.
You accidentally changed compilation settings of 1 file out of a hundred? Good luck figuring out which one. Go over all files in UI one by one.
In other words: managing project files using Visual Studio UI is a nightmare.
Premake is a solution. It’s a meta-build system. You define your build using lua scripts, which look like test configuration files.
Premake then can generate Visual Studio projects, XCode project, makefiles etc. That’s the meta part.
It was truly a life server on project with lots of files (SumatraPDF’s own are over 300, many times more for third party libraries).
Using /analyze and cppcheck
cppcheck and
/analyze flag in
cl.exe are tools to find bugs in C++ code via static analysis. They are like a C++ compiler but instead of generating code, they analyze control flow in a program to find potential programs.
It’s a cheap way to find some bugs, so there’s no excuse to not run them from time to time on your code.
Using asan builds
Address Sanitizer (
asan) is a compiler flag
/fsanitize=address that instruments the code with checks for common memory-related bugs like using an object after freeing it, over-writing values on the stack, freeing an object twice, writing past allocated memory.
The downside of this instrumentation is that the code is much slower due to overhead of instrumentation.
I’ve created a project for release build with asan and run it occasionally, especially in stress test.
Write for the debugger
Programmers love to code golf i.e. put us much code on one line as possible. As if lines of code were expensive.
Many would write:
Bad:
// ...
return (char*)(start + offset);
I write:
Good:
// ...
char* s = (char*)(start + offset);
return s;
Why?
Imagine you’re in a debugger stepping through a debug build of your code.
The second version makes it trivial to set a breakpoint at return s line and look at the value of s. The first doesn’t.
I don’t optimize for smallest number of lines of code but for how easy it is to inspect the state of the program in the debugger.
In practice it means that I intentionally create intermediary variables like s in the example above.
Do it yourself standard library
I’m not using STL. Yes, I wrote my own string and vector class. There are several reasons for that.
Historical reason
When I started SumatraPDF over 15 years ago STL was crappy.
Bad APIs
Today STL is still crappy. STL implementations improved greatly but the APIs still suck.
There’s no API to insert something in the middle of a string or a vector. I understand the intent of separation of data structures and algorithms but I’m a pragmatist and to my pragmatist eyes v.insert (v.begin(), myarray, myarray+3); is just stupid compared to v.inert(3, el).
Code bloat
STL is bloated. Heavy use of templates leads to lots of generated code i.e. surprisingly large binaries for supposedly low-level language.
That bloat is invisible i.e. you won’t know unless you inspect generated binaries, which no one does.
The bloat is out of my control. Even if I notice, I can’t fix STL classes. All I can do is to write my non-bloaty alternative, which is what I did.
Slow compilation times
Compilation of C code is not fast but it feels zippy compared to compilation of C++ code.
Heavy use of templates is big part of it. STL implementations are over-templetized and need to provide all the C++ support code (operators, iterators etc.).
As a pragmatist, I only implement the absolute minimum functionality I use in my code.
I minimize use of templates. For example Str and WStr could be a single template but are 2 implementations.
I don’t understand C++
I understand the subset of C++ I use but the whole of C++ is impossibly complicated. For example I’ve read a bunch about std::move() and I’m not confident I know how to use it correctly and that’s just one of many complicated things in C++.
C++ is too subtle and I don’t want my code to be a puzzle.
Possibility of optimized implementations
I wrote a StrVec class that is optimized for storing vector of strings. It’s more efficient than std::vector<std::string> by a large margin and I use it extensively.
Temporary allocator and pool allocators
I use temporary allocators heavily. They make the code faster and smaller.
Technically STL has support for non-standard allocators but the API is so bad that I would rather not.
My temporary allocator and pool allocators are very small and simple and I can add support for them only when beneficial.
Minimize unsigned int
STL and standard C library like to use size_t and other unsigned integers.
I think it was a mistake. Go shows that you can just use int.
Having two types leads to cast-apalooza. I don’t like visual noise in my code.
Unsigned are also more dangerous. When you substract you can end up with a bigger value. Indexing from end is subtle, for (int i = n; i >= 0; i--) is buggy because i >= 0 is always true for unsigned.
Sadly I only realized this recently so there’s a lot of code still to refactor to change use of size_t to int.
Mostly raw pointers
No std::unique_ptr for me.
Warnings are errors
C++ makes a distinction between compilation errors and compilation warnings.
I don’t like sloppy code and polluting build output with warning messages so for my own code I use a compiler flag that turns warnings into errors, which forces me to fix the warnings.
