BewareMyPower opened a new pull request, #64:
URL: https://github.com/apache/pulsar-client-cpp/pull/64

   Fixes https://github.com/apache/pulsar-client-cpp/issues/60
   
   ### Motivation
   
   The includes in pulsar-client-cpp is very casual. There are a lot of 
implicit includes and the forward declaration is not used much. For example, if 
`lib/ClientConnection.h` was modified, 27 files would be recompiled.
   
   The other problem is the `SortIncludes` attribute in `.clang-format` file is 
false. It might be okay in early days. However, as the project grows, including 
many headers without ordering brings a very bad experience. Combining with the 
very few usages of the forward declaration, it's hard to determine whether a 
header is still required after a change.
   
   ### Modifications
   
   Apply forward declarations as much as possible and change the `SortIncludes` 
to true in `.clang-format` file.
   
   It's special for the `PulsarApi.pb.h` file because the size of this header 
is over 1 MiB. For classes we can use forward declaration, but for enumerations 
we have to include this header.
   
   To solve this problem, `ProtoApiEnums.h` is added to define some constant 
integers that can be cast implicitly from the enumerations. If we want to use 
enumerations from `PulsarApi.pb.h`, we can include `ProtoApiEnums.h` instead.
   
   Finally, to unify the include rules, in `lib/*.h`, if we want to include a 
header (e.g. `xxx.h`) from the same directory, just use `#include "xxx.h"`. 
Don't use `#include "lib/xxx.h"` or `#include <lib/xxx.h>` in headers of `lib/` 
directory.
   
   ### Improvements
   
   Since forward declaration is applied everywhere now, take 
`lib/ClientConnection.h` for example. After this patch, only 10 files needs to 
be recompiled, while 27 files would be recompiled before.
   
   This patch also reduces the binary size and speeds up the compilation time.
   
   Binary size:
   - `libpulsar.a`: 319234696 (305 MiB) -> 286716564 (274 MiB)
   - `libpulsar.so`: 110162496 (106 MiB) -> 102428456 (98 MiB)
   
   Compilation time with the following commands:
   
   ```bash
   cmake -B build -DBUILD_TESTS=OFF
   cmake --build build -j8
   ```
   
   From 2m35.640s to 1m44.213s.
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc-required` 
   (Your PR needs to update docs and you will update later)
   
   - [x] `doc-not-needed` 
   (Please explain why)
   
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already added)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to