pugixml icon indicating copy to clipboard operation
pugixml copied to clipboard

Implement <xml_memory_pool> interface for custom allocators

Open Ambal opened this issue 5 years ago • 3 comments
trafficstars

The <xml_memory_pool> interface is a simplified version of the C++17's std::pmr::memory_resource. Allows construction of pugi::xml_document/pugi::xpath_query objects using preallocated memory buffers. Two specific memory pools are provided:

  1. xml_monotonic_memory_pool - similar to std::pmr::monotonic_memory_resource;
  2. memory_resource_adapter<T> - wrapper for classes which implement interface similar to std::pmr::memory_resource;

Some limitations:

  1. <xml_memory_pool> cannot be used with all xpath-related classes due to binary incompatibility, only <xpath_query> is supported;
  2. <xpath_query> swaps memory pools in move assignment operator since functionality to clone internal structures is missing;

Ambal avatar Dec 26 '19 21:12 Ambal

hi, @zeux . I guess something is not right with code coverage since codecov argues about the code in the convert_buffer() method for PUGIXML_WCHAR_MODE code path. It seems to be not covered by existing unit-tests

Ambal avatar Dec 26 '19 23:12 Ambal

memory_resource_adapter seems a bit too complex - std::pmr::memory_resource seems to guarantee alignment for max_align_t which should be enough to satisfy the alignment requirements. Do we need to spend extra memory / code to perform the alignment here?

I will need to take a look at the rest of the change. Can you elaborate on the code coverage issue?

zeux avatar Jan 01 '20 19:01 zeux

Hi, @zeux. The complexity of <memory_resource_arapter> is due to the extra 'size' parameter of the "deallocate" function used by std::pmr::memory_resource - we don't store the allocated size anywhere as opposed to STL classes. Also, we don't perform any alignment for std::pmr::memory_resource - we simply provide the alignment being used by internal pugixml data types as-is. We align buffer ourselves only in <xml_monotonic_memory_pool> class' code.

The code coverage issue is straightforward - the unit-test suite seems to be missing the test case for the function being altered by my code change e.g. the UTF32 buffer parsing.

Ambal avatar Jan 01 '20 22:01 Ambal