xlnt-community / xlnt / 8cf07b4b-80d1-4792-a6ab-0460b2e091f2 / 1

84%

master: 84%

push

circleci

Fix garbage collection of formats (#115)

`format_impl_ptr` is now used instead of a `format_impl*`. All reference
counting is delegated to format_impl_ptr.

`format_impl_ptr` triggers garbage collection when reference count drops
to zero. Previously, garbage collection was run too often, resulting in
a significant overhead when reading many formats (see benchmark-styles).

`format_impl` is owned by the stylesheet iff `format_impl::stylesheet
!== nullptr` If the format is still referenced while the stylesheet is
cleared/destructed, ownership is delegated to `format_impl_ptr` by
setting `format_impl::stylesheet = nullptr`. This may occur when a
`xlnt::format` object still exists when the workbook is cleared.

`format_impl_ptr` implements a custom smart pointer, instead of using
the traditional `std::shared_ptr` / `std::weak_ptr` approach, to allow
better control over the garbage collection process. Garbage collection
should not run when parsing an xlsx file, even not after the document is
fully read, to support a successful round trip, preserving even unused
formats. See the many `round_trip_matches_rw` tests.

`xlnt::format`: return a reference to this, instead of creating
unnecessary copies. The intention seemed to be to create a fluent
interface, but this requires a reference to be returned, as the
`xlnt::format` copy constructor performs a deep (lazy) copy. It's not a
wrapper like e.g. `xlnt::cell.`

`xlnt::detail::references`: implements reference counting for an object.
Copy constructor does not copy the count to the new object, as it is a
new object requiring its own reference counting.

`xlnt::workbook::empty`: `create_format(true)` returns a copy of the
created format. Modifying the returned format, doesn't actually change
the default format itself. Previously this was the case, due to quirks
in the reference counting mechanism.

Fixes #93

14324 of 18832 branches covered (76.06%)

11763 of 14247 relevant lines covered (82.56%)

10760.65 hits per line