SmartCursors and SmartRanges in KDE 4.0 – KDE 4.4
Since KDE 4.0 the KTextEditor interfaces have so called SmartCursors and SmartRanges. A SmartCursor is a text cursor (i.e. a line/column tuple) , which is bound to a text document. When editing the text document, the cursor is automatically moved such that it maintains its position. You need this for the displayed text cursor in a view for instance. If you type text, the cursor automatically moves.
A SmartRange consists of two SmartCursors: start and end. We use that for instance for the text selection or the inline spell checking, or KDevelop uses it to add arbitrary highlighting to parsed C/C++ code. Again, if you modify text in the document, the text range automatically moves, expands or shrinks.
The concept of SmartCursors and SmartRanges is doubtless very useful. However, for KDE 4.5 the Kate developers came up with an alternative implementation that will even deprecate the SmartCursors and SmartRanges in KDE 4.6. The reason for this is that the current implementation has several problems, which we will discuss in the following.
The SmartRanges API can be used in very wrong ways. For instance, the SmartInterface has the two functions deleteCursors() and deleteRanges(). Besides that, a document reload also deletes all SmartCursors and SmartRanges. Both cases lead to a dangling pointer if you use SmartCursors or SmartRanges. Hence, whenever you use them, you always need a so-called SmartCursorWatcher / SmartCursorNotifier and SmartRangeWatcher / SmartRangeNotifier, which tell you that your SmartCursor or SmartRange is about to be deleted. Watcher and Notifier basically do the same thing. Besides deletion notification, they also tell you more e.g. that the SmartCursor position changed. If you use a notifier, those notifications are sent via signals. If you use a watcher, you have to overwrite virtuals. That is, we have two different concepts that basically do the same thing. If you use thousands of SmartRanges e.g. for arbitrary highlighting, we have possibly thousands of those emits, which does not really scale.
The API also allows to have parent/child relations, i.e. a SmartRange can have children and a parent. It is possible to traverse this hierarchy, and again since you get the pointers you can delete arbitrary SmartRanges or SmartCursors. Another reason why you always need a notifier or watcher. And if you have a notifier/watcher, you always have the signals emitted when e.g. a cursor changes, as explained above.
Further, we have lots of places where we use reference object KTextEditor::Cursor& as parameter. This const reference-object signals that it does not change. But due to SmartRange deriving from KTextEditor::Cursor, this object might also be a SmartCursor. So despite of being declared as a const object, it may change its position behind your back. This may lead to unexpected behaviours in e.g. loops where the line or column of the cursor is assumed to be constant. This is a problem in almost all functions in KatePart, so passing SmartCursors as Cursors is a very bad idea, but of course allowed.
As mentioned above, you always need a Notifier or Watcher for all SmartCursors and SmartRanges. This will emit lots of signals you probably rarely need. Still, there is no way around it. This is a huge overhead, it simply does not scale.
The implementation is really complex and rather undocumented in most areas. Unfortunately, only very few (or maybe even no one) really understand the code. Besides that, it is very fragile. If you change something, you might break the code. Hence, we often heard similar comments like “don’t touch the code, otherwise it will break”. Such comments alone already indicate that the current implementation is not maintainable. Unmaintainable code is bad, especially in open source projects (this is not a good way to gain new developers at all). There are other downsides of the current implementation: SmartCursors need a lot of memory for each instance; there are lots of features like animations for SmartRanges, which make the code even more complex; redundancy of watchers and notifiers bloat the code and influence the runtime behavior.
It seems there was the idea of making the KTextEditor interfaces thread safe. The SmartRanges interact with the content of the document, e.g. querying lines count and line length of lines in the text buffer. As this is done by other threads we need correct locking in all places: in document content changes, in smart cursors, in smart ranges, in the painting routines, etc. The current state in KatePart is that not all functions do the right locking. That’s why we have lots of asserts/crashs. KatePart already has more than 150 lockings at the moment, but they still do not voer problems. And very few developers (or no one?) really know when to lock and when not to. This is especially complex since we want to ensure that the locks are not hold while emitting signals or calling functions provided from the outside as callbacks, this is still not done completely right, too.
If you think about Qt, the GUI widgets are also not thread-safe, they all live in the main thread. And if you need data in other threads, you always use queued signals. This is pretty much what we experience in KatePart now. Not enough locking? Too much locking? In other words: It’s far too complex to make the KTextEditor interfaces thread-safe…
Threading and Revisions
Now to another issue in the implementation. KDevelop uses KTextEditor::Ranges in the parsing thread. Think of the following use case: KDevelop gets all the document text in the current revision (or version, if you prefer). Now it takes some time until the parsing is finished. When it’s finished, KDevelop uses the Ranges (which initially belong to the old revision of the document), and then translates those ranges to the current version of the text document (this is simple: Just track all edits inside KatePart (delete, insert, wrap, unwrap) and apply those transformations to the ranges). Now we transformed ranges from an old text revision to the current text revision. This means KDevelop does not have to parse everything again, as it knows exactly which parts changed. Awesome indeed 🙂 However, now comes the problem: To transform Cursors and Ranges between revisions, you have to tell the SmartInterface about the revision you are working with. This is done via SmartInterface::useRevision(int). The API documentation says: Tell the smart interface to work against the given revision when creating cursors and ranges. That is, if you call useRevision() once, all succeeding calls like newSmartRange() etc are working in the past (at that revision). Also kind of cool, since useRevision() works locally in each thread. That is different threads don’t influence each other. But there is huge problem with this: Think of a KTextEditor user (KDevelop, Kile, …) uses useRevision() in the main thread. Then all succeding calls of e.g. newSmartRange() use an old revision instead of the current one. Hence, KatePart itself is completely broken in that case. (This case is now catched via a qFatal() in KatePart). But again this shows that multi-threading simply complicates the matter a lot. It would have been much easier to say transformRange(int fromRevision, int toRevision) instead of just one translateFromRevision() that translates the given range against the revision specified through useRevision(). Hence, the current API is unfortunately pretty much broken by design.
A necessary Step?
Often it’s hard to get things done right in the first try. So maybe the issues above are a necessary step to a much better implementation. And that is exactly what happened in the last two months: We have new interfaces called MovingInterface, MovingCursor and MovingRange. For feedback there is MovingRangeFeedback, which uses callbacks for notification (no signals by intention). All those classes have unit tests. KatePart in KDE 4.6 will not implement any Smart* interface anymore. Hence, KatePart will be completely single threaded again. We already started to remove several thousands of lines Smart* code that was never used/finished. Much more will follow immediately after the KDE 4.5 release. This means all developers using Smart* from the KTextEditor interfaces should migrate to the Moving* pendents for KDE 4.5 already, if possible.
I’ll provide very detailed information about the Moving* implementation in further blogs during the next month.