KristopherGBaker / Maaku

The Maaku framework provides a Swift wrapper around cmark with the addition of a Swift friendly representation of the AST
MIT License
51 stars 12 forks source link

Should be able to move nodes from one document to another #46

Open NightFlyer opened 5 years ago

NightFlyer commented 5 years ago

If Maaku allows for manipulating the AST, then it should also be possible to move a subtree from one document to another.

This is possible with the underlying cmark code (it's all just pointers and linked lists), but it has implications for the way memory is managed in Maaku.

The simplest way to fix this in Maaku is to have a node's parent actually be a strong reference to the parent CMNode, with the top level document node having no parent. The top level document node is then referenced by every node within the tree, and will only be deallocated when all parts of the subtree are gone, and then it can release the memory (which causes the libcmark code to walk back through the subtree releasing the actual cmarkNode memory). However, this will increase memory usage.

Currently, each node points to a single node that controls memory releasing, which means that parent nodes up the tree can be freed if not needed. So, by actually keeping the parent for each node, we will hold more items in memory.

As an example, take a tree that has 5 levels like:

  1
 /  \
2   3
      |
     4
    /  \
  5    6
 /  \
7   8

Under the current code, if I run an iteration on node 1 that remembers node 7, the only nodes that are still in memory after the iteration are nodes 1 and 7 -- all the other CMNode objects are simply created and then released.

If I then run another iteration on node 1 and remember node 8, then nodes 1, 7 and 8 will still be in memory.

If I finally run another iteration on node 1 and remember node 7 again, then I'll have objects for node 1, two for node 7 (since the iteration code creates new objects as it walks the tree), and the object for node 8.

If we change to holding the parent node strongly for each node, rather than all nodes referencing the one memory holder, then the above example would result in:

This is because each iteration creates new wrapper nodes for all the items below the node that started the iteration.

Note that because iteration always creates new wrapper nodes, it's not possible to keep the current memory management system and allow for a node to be moved from one document to another without introducing the possibility of memory crashes. An alternative to the "strong parent" method would be to maintain a dictionary of nodes currently in memory, and to use that rather than always creating new wrapper nodes. That, however, has implications of performance if we want it to be threadsafe.

NightFlyer commented 5 years ago

After more thought, the only way to have this happen while keeping true memory safety is to keep track of CMNodes (most likely by keeping them unique and in a 1 to 1 relationship with the underlying cmark_node).

Probably the safest thing to do is to provide a way to copy a subtree from one document into another document.