Add profiling code

[bobfang1992]

This is to verify some of the previous observation made here. The code seems to be spending most time converting types from python to toml and vice versa.

In [9]: print(pytomlpp.get_profiling_stats_summary())                                                                                                                                                                                                                                                                                
Summary of Profiling:
loads.convert : (counter = 100, total_time_in_ms = 0.120558, average_time_in_ms = 0.00120558)
loads.total : (counter = 100, total_time_in_ms = 0.111418, average_time_in_ms = 0.00111418)

From the above stats it seems we spend more than 80% merely converting types, which I believe we can do better.

This code may be rough but I believe this is a good starting point to add some insights into the performance of this library.

Some thoughts:

1. should I make the profiling functions private by adding __ to them so they are not directly exposed to user?
2. is there any way to make the profiling less intrusive? I believe currently even when the profiling is disabled the code still calls the constructor and deconstructor of profiling_guard.

3. I noticed some weird behaviour listed below. Seems the profiling code is counting more time on convert than in total. Does this suggest that there is a bug I do not see in it? Curious to hear your thoughts.

   In [9]: print(pytomlpp.get_profiling_stats_summary())                                                                                                                                                                                                                                                                                 
   Summary of Profiling:
   loads.convert : (counter = 100, total_time_in_ms = 0.120558, average_time_in_ms = 0.00120558)
   loads.total : (counter = 100, total_time_in_ms = 0.111418, average_time_in_ms = 0.00111418)

[bobfang1992]

@EpicWink @chaitan94 @marzer What's your thoughts?

[marzer]

Well if you have to do the profiling python-side I'm not sure how you could make it less intrusive, really. What you have here is going to be negligible when contrasted with the cost of the pybind11 machinery anyways, so I wouldn't worry about that too much.

RE performance of the conversion code itself, there's a few things I'd look at:

Copying objects during iteration

https://github.com/bobfang1992/pytomlpp/blob/fb56a985b5c720d6a2422fa4f54e0abcb792a13e/include/pytomlpp/pytomlpp.hpp#L158

What is the type of it here? Is it a trivial type like a pointer, or some object? If it's a non-trivial object, this is doing a copy on every iteration, which I imagine might have some impact given pybind's reference-counting semantics etc. Consider for (auto& it : list) instead.

Copying key-value pairs during iteration

https://github.com/bobfang1992/pytomlpp/blob/fb56a985b5c720d6a2422fa4f54e0abcb792a13e/include/pytomlpp/pytomlpp.hpp#L207-L209

Same as above, only this time you're not only potentially copying the KVP object itself, but the key and value objects themselves, and then explicitly copying them again immediately after to give them names. Given that this is C++17 you can actually solve that problem and clean up the syntax by using a structured binding instead, i.e. for (auto& [key, value] : object)

Copying when moving is an option

https://github.com/bobfang1992/pytomlpp/blob/fb56a985b5c720d6a2422fa4f54e0abcb792a13e/include/pytomlpp/pytomlpp.hpp#L104-L107

The code creates a const string by copying from the toml data, then copies again in the assignment. You could eliminate one of those copies by doing a std::move:

      const toml::value<std::string> *string_value = value->as_string();
      std::string string_v = string_value->get(); //non-const
      result[key] = std::move(string_v);

(if you made the function take the input as toml::table&& you could even skip the copy entirely and only do a move from the source table, though I don't know how/where toml_table_to_py_dict is called to comment on if that's an option for you)

Use visit and/or switch

More generally, you can restructure those functions to use a switch on the value type instead of calling value->type() repeatedly in an if/else, or even simpler, refactor it to use toml::node::visit() instead, which handles all the switch machinery for you internally (example here).

[bobfang1992]

@marzer thanks for the advice. I've created a new issue #32 so I can implement these later (especially using the visitor pattern). This is very useful.

[EpicWink]

I would suggest hiding all of the profiling code behind a compiler macro (order etc), such as PYTOMLPP_ENABLE_PROFILING, and make it print to stderr instead of returning