Simple NAPI wrapper around self written C++ msgpack, horrible performance

Question

I have written a relatively small msgpack implementation in C++, with performance that is not too bad, so I decided to write a NAPI wrapper around it, but the performance is just horrible. I am guessing this is because I'm having to find the type of everything before being able to pack it, and the performance of Napi::Array and Napi::Object.

#include <napi.h>
#include <cstdint>
#include <string>
#include <vector>
#include "msgpack.hpp"

namespace node_msgpack {
    using namespace std;
    using namespace Napi;

    bool is_int(Env& env, Value& num) {
        return env.Global().Get("Number").ToObject().Get("isInteger").As<Function>().Call({ num }).ToBoolean().Value();
    }

    void pack(Env& env, vector<Value>& src, msgpack_byte::container& dest, bool initial = true);

    void pack(Env& env, Value& src, msgpack_byte::container& dest, bool initial = false) {
        switch (src.Type()) {
            case napi_boolean: {
                msgpack::pack(src.ToBoolean().Value(), dest);
                break;
            }
            case napi_number: {
                if (is_int(env, src)) {
                    int64_t num = src.ToNumber().Int64Value();
                    if (num >= 0) {
                        msgpack::pack_uint(num, dest);
                    }
                    else {
                        msgpack::pack_int(num, dest);
                    }
                }
                else {
                    msgpack::pack(src.ToNumber().DoubleValue(), dest);
                }
                break;
            }
            case napi_string: {
                msgpack::pack(src.ToString().Utf8Value(), dest);
                break;
            }
            case napi_null:
            case napi_undefined: {
                msgpack::pack(nullptr, dest);
                break;
            }
            case napi_object: {
                if (src.IsArray()) {
                    Array input_js_arr = src.As<Array>();
                    const uint32_t n = input_js_arr.Length();
                    vector<Value> input_vec;
                    input_vec.resize(n);
                    for (uint32_t i = 0; i < n; i++) {
                        input_vec[i] = (input_js_arr.Get(i));
                    }
                    pack(env, input_vec, dest);
                }
                else {
                    Object input_js_obj = src.As<Object>();
                    Array obj_props = input_js_obj.GetPropertyNames();
                    const uint32_t n = obj_props.Length();
                    if (initial) {
                        dest.check_resize(size_t(msgpack::LengthOf(src) * compression_percent));
                    }
                    if (n <= 15) {
                        dest.push_back(fixmap_t(n));
                    }
                    else if (n <= umax16) {
                        dest.push_back(uint8_t(map16));
                        dest.push_back(uint16_t(n));
                    }
                    else if (n <= umax32) {
                        dest.push_back(uint8_t(map32));
                        dest.push_back(uint32_t(n));
                    }
                    for (uint32_t i = 0; i < n; i++) {
                        Value key = obj_props.Get(i);
                        Value val = input_js_obj.Get(key);
                        pack(env, key, dest, false);
                        pack(env, val, dest, false);
                    }
                    if (initial) {
                        dest.shrink_to_fit();
                    }
                }
                break;
            }
        }
    }

    void pack(Env& env, vector<Value>& src, msgpack_byte::container& dest, bool initial) {
        size_t n = src.size();
        if (initial) {
            dest.check_resize(size_t(msgpack::LengthOf(src) * compression_percent));
        }
        if (n <= 15) {
            dest.push_back(fixarray_t(n));
        }
        else if (n <= umax16) {
            dest.push_back(uint8_t(arr16));
            dest.push_back(uint16_t(n));
        }
        else if (n <= umax32) {
            dest.push_back(uint8_t(arr32));
            dest.push_back(uint32_t(n));
        }
        for (uint32_t i = 0; i < n; i++) {
            pack(env, src[i], dest, false);
        }
        if (initial) {
            dest.shrink_to_fit();
        }
    }

    String stringify(const CallbackInfo& info) {
        Env env = info.Env();
        msgpack_byte::container dest;
        if (info.Length() == 0) {
            Error::New(env, "Expected an argument").ThrowAsJavaScriptException();
            return String::New(env, "");
        }
        Value input = info[0];
        pack(env, input, dest, true);
        return String::New(env, msgpack_byte::to_string(dest));
    }
}

Napi::Object Init(Napi::Env env, Napi::Object exports) {
    exports.Set(Napi::String::New(env, "stringify"), Napi::Function::New(env, node_msgpack::stringify));
    return exports;
}

NODE_API_MODULE(addon, Init)

The msgpack::pack, msgpack::pack_uint and msgpack::pack_int calls the pack function to the plain C++ msgpack library I wrote https://github.com/t348575/msgpack

Am I using the most efficient method to check types? I tried using visual benchmarking like shown here, but I could not get anything useful out of it. To compare, the napi wrapper takes around 20 seconds to stringify ~400MB of data whereas the plain C++ takes only 0.4s to do the same. The entire code can be found here

Answer 1

Profiling:

We could use a library like Tracy to do the sort of profiling you mention in the question. But it's quicker to ask "what's taking the most time" and get an answer with a profiler like Very Sleepy, or the built in Visual Studio profiler.

So... I stuck your code into a project created with create-vs-napi, and ran stringify on some json data (~2MB) I had lying around. Obviously this kind of testing depends a lot on the actual data (I'm testing a scene graph type structure, with a fair mix of objects, arrays, strings and numbers).

First off, let's add an overall timer to check how long the code is taking:

String stringify(const CallbackInfo& info) {

    auto start = std::chrono::high_resolution_clock::now();

    Env env = info.Env();
    msgpack_byte::container dest;
    if (info.Length() == 0) {
        Error::New(env, "Expected an argument").ThrowAsJavaScriptException();
        return String::New(env, "");
    }
    Value input = info[0];
    pack(env, input, dest, true);
    String result = String::New(env, msgpack_byte::to_string(dest));

    auto duration = start - std::chrono::high_resolution_clock::now();
    auto duration_ms = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();

    std::cout << "time: " << duration_ms << "ms\n";

    return result;
}

For my data that's somewhere around 800 milliseconds.

---

So lets run it under the Visual Studio profiler (Debug->Performance Profiler->CPU Usage).

This just tells us that we're spending a lot of time in memcpy and the pack functions. If we expand the looooong tree of calls we get to the stringify function, and can look at what's actually taking the time.

If we click "Open Details..." on the main profiling page, and "Expand Hot Path", we get the following:

This shows the sequence of pack calls taking all the time. If we click on each function call, we can see the source code in the bottom window, with a breakdown of the percentages taken by each line of code (bearing in mind that it's an optimized release build). For example, the stringify function is shown above.

We can see that there are a couple of pack functions that take longer than the rest:

• node_msgpack::pack (84.68%): 17.91% of the execution time is in calling dest.push_back(fixmap_t(n));. If we expand it, we see that the time is almost all spent in the std::copy in msgpack_byte::container::check_expand().

• node_msgpack::pack (62.96%): 19.33% of the time is spent in msgpack::pack for strings... specifically in msgpack_byte::container::check_expand().

• node_msgpack::pack (41.88%): 36% of the time is spent in dest.shrink_to_fit() (which uses memcpy).

• node_msgpack::pack (18.65%): spends all its time in container::check_expand() again.

So... it looks like we're spending a lot of time expanding the container (and hence copying elements after reallocation). Then we call shrink_to_fit() and shrink the container (and copy all the elements again)!

---

So let's not do that!

I decided to delete all the calls to shrink_to_fit(). I also figure that we can let push_back expand the container when it needs to, so I deleted the initial arguments and the calls to check_resize().

Now stringify takes around 100 milliseconds. Much better.

---

If we profile it again, we get:

So we can see that ~20% of the program execution time (which is half of entire time spent in stringify) is spent in node_msgpack::is_int:

bool is_int(Env& env, Value& num) {
    return env.Global().Get("Number").ToObject().Get("isInteger").As<Function>().Call({ num }).ToBoolean().Value();
}

This doesn't look like a fast function. (Looking stuff up by string, manual type casting and function composition?) I have no idea if there's a quicker way to do the same thing with Napi. It's being called from:

    case napi_number: {
        if (is_int(env, src)) {
            int64_t num = src.ToNumber().Int64Value();
            if (num >= 0) {
                msgpack::pack_uint(num, dest);
            }
            else {
                msgpack::pack_int(num, dest);
            }
        }
        else {
            msgpack::pack(src.ToNumber().DoubleValue(), dest);
        }
        break;
    }

As I understand it, in javascript all numbers are 64 bit floating point. So since we're going to be unpacking them as doubles anyway, the only advantage of this would be saving some space in the message.

If this is worthwhile, perhaps it's something that the msgpack library should do internally to all double values (or have a separate function to call with floating point values that probably represent integers).

It would almost certainly be quicker to check the value of the double in C++ instead of calling the javascript isInteger function.

For now let's just write doubles, and see what happens:

    case napi_number: {
        msgpack::pack(src.ToNumber().DoubleValue(), dest);
        break;
    }

Now stringify takes ~50 milliseconds!

That's probably good enough for now, but you could definitely take the profiling further.

---

Review:

using namespace is bad practice:

using namespace std;
using namespace Napi;

This is a bad habit to get into, because it can result in name collisions and other problems.

(It also makes it harder for someone with no knowledge of Napi (like me) to understand the code... is that Array class a Napi thing or not? I just have to guess.)

It's much better to type std:: or Napi:: where necessary.

---

avoid unnecessary inefficiency:

            const uint32_t n = input_js_arr.Length();
            vector<Value> input_vec;
            input_vec.resize(n);
            for (uint32_t i = 0; i < n; i++) {
                input_vec[i] = (input_js_arr.Get(i));
            }
            pack(env, input_vec, dest);

We could easily write the length and then iterate through the values directly here (as we do with a map), instead of copying to a temporary vector. Then we'd only need a single pack function.

---

function signature:

void pack(Env& env, Value& src, msgpack_byte::container& dest)

If we delete is_int then we don't use env anywhere else, so we can remove this argument. Also src can be passed by const&.

---

use std::vector:

The msgpack library is out of scope of this review, but it should almost certainly use std::vector, instead of rolling its own container type.

As a side-note, you might find that 2x is too large an expansion factor for a container, and 1.1x is too small. 1.5x is fairly standard.