{"action":"created","issue":{"url":"https://api.github.com/repos/ckeditor/ckeditor5-core/issues/62","labels_url":"https://api.github.com/repos/ckeditor/ckeditor5-core/issues/62/labels{/name}","comments_url":"https://api.github.com/repos/ckeditor/ckeditor5-core/issues/62/comments","events_url":"https://api.github.com/repos/ckeditor/ckeditor5-core/issues/62/events","html_url":"https://github.com/ckeditor/ckeditor5-core/issues/62","id":113792495,"number":62,"title":"Nodes list compression","user":{"login":"pjasiun","id":1166967,"avatar_url":"https://avatars.githubusercontent.com/u/1166967?v=3","gravatar_id":"","url":"https://api.github.com/users/pjasiun","html_url":"https://github.com/pjasiun","followers_url":"https://api.github.com/users/pjasiun/followers","following_url":"https://api.github.com/users/pjasiun/following{/other_user}","gists_url":"https://api.github.com/users/pjasiun/gists{/gist_id}","starred_url":"https://api.github.com/users/pjasiun/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pjasiun/subscriptions","organizations_url":"https://api.github.com/users/pjasiun/orgs","repos_url":"https://api.github.com/users/pjasiun/repos","events_url":"https://api.github.com/users/pjasiun/events{/privacy}","received_events_url":"https://api.github.com/users/pjasiun/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/ckeditor/ckeditor5-core/labels/module:treemodel","name":"module:treemodel","color":"fef2c0"}],"state":"open","locked":false,"assignee":{"login":"scofalik","id":1502228,"avatar_url":"https://avatars.githubusercontent.com/u/1502228?v=3","gravatar_id":"","url":"https://api.github.com/users/scofalik","html_url":"https://github.com/scofalik","followers_url":"https://api.github.com/users/scofalik/followers","following_url":"https://api.github.com/users/scofalik/following{/other_user}","gists_url":"https://api.github.com/users/scofalik/gists{/gist_id}","starred_url":"https://api.github.com/users/scofalik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scofalik/subscriptions","organizations_url":"https://api.github.com/users/scofalik/orgs","repos_url":"https://api.github.com/users/scofalik/repos","events_url":"https://api.github.com/users/scofalik/events{/privacy}","received_events_url":"https://api.github.com/users/scofalik/received_events","type":"User","site_admin":false},"milestone":null,"comments":1,"created_at":"2015-10-28T11:00:01Z","updated_at":"2016-01-13T13:55:39Z","closed_at":null,"body":"Because every character is now a separate class, tree model can consume a lot of memory. The situation is event worse because model will keep the history of changes including all historical nodes and operations with lists of nodes to change.\r\n\r\nThe solution is to introduce compression inside `NodeList` class which is used for both operations and child list. The idea is to keep existing interface with characters as children, but to implement it internally in the way that all siblings characters with the same attributes will be stored as as a single `Text` (string with attributes). \r\n\r\nThen characters may be a proxy witch act like a single character, but have reference to the whole text and modify it if characters attribute changes. The character class does not need to be lightweight anymore, because there will not be many references to these classes kept in the code.\r\n\r\nIf user call `getChild` at the middle of the string the new character class will be created with the reference to that string. Disadvantage would be that it will not be possible to simple compare references to two characters, but is does not seems to be a common operation. On the other hand, it will be possible to get `Position` before/after both character and compare them.\r\n\r\nThis mechanism needs to be smart. The most common operations should work fast, without creating a special API for them. For example consider changing attribute on the set of following characters (like applying bold on the range). The change attribute for the character should check if the new list of attributes is the same as the list of attributes of the previous text and move that character to the previous text. Special method to apply style on the range would work faster, but having separate API to work on whole text nodes is something we want to avoid, at least at the begging.\r\n\r\nThanks to this compression we will not only save memory because of having less character classes, but also reduce number of attributes and maybe be able to change the attribute data structure, which is getting strange and slow because of decisions to save memory."},"comment":{"url":"https://api.github.com/repos/ckeditor/ckeditor5-core/issues/comments/171297452","html_url":"https://github.com/ckeditor/ckeditor5-core/issues/62#issuecomment-171297452","issue_url":"https://api.github.com/repos/ckeditor/ckeditor5-core/issues/62","id":171297452,"user":{"login":"scofalik","id":1502228,"avatar_url":"https://avatars.githubusercontent.com/u/1502228?v=3","gravatar_id":"","url":"https://api.github.com/users/scofalik","html_url":"https://github.com/scofalik","followers_url":"https://api.github.com/users/scofalik/followers","following_url":"https://api.github.com/users/scofalik/following{/other_user}","gists_url":"https://api.github.com/users/scofalik/gists{/gist_id}","starred_url":"https://api.github.com/users/scofalik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scofalik/subscriptions","organizations_url":"https://api.github.com/users/scofalik/orgs","repos_url":"https://api.github.com/users/scofalik/repos","events_url":"https://api.github.com/users/scofalik/events{/privacy}","received_events_url":"https://api.github.com/users/scofalik/received_events","type":"User","site_admin":false},"created_at":"2016-01-13T13:55:39Z","updated_at":"2016-01-13T13:55:39Z","body":"Here is the new behavior of `NodeList`:\r\n\r\nWe have `Text` class, which is only for representing a string with attached attributes to it. It is mostly used to append characters with set attributes to `Element` instances. `Text` is not a `Node` class.\r\n\r\nWe got rid of `Character` class and introduced similiar `TextNode` class. `TextNode` is extending `Node` class. It is used as a representation of some text in tree model. This class is a bit weird but is a best consensus between user expectations, code efficiency, API complication and amount of time spent coding it.\r\n\r\n`NodeList` does not contain singular characters like before. Now all consecutive characters with same attributes are kept as one instance of `Text` class. So internal `NodeList` array is keeping instances of `Element` and `Text` inside it. Still, outside, characters are visible as singular beings. So if your `NodeList` contain a `P` element, `foo` string and `DIV` element if you iterate over it (without merging characters) you get `P` element, `f` text node, `o` text node, `o` text node, `DIV` element. Similary, when you use `nodeList.get( 3 )` you will get `o` text node.\r\n\r\n`TextNode` is kind of a \"readonly proxy\" to the characters that are contained in the `NodeList`. It looks like a `Node` with some exceptions. Most notably, any changes done to `TextNode` are NOT reflected on original `Text` object. This is mostly okay - all in all changes to tree model should be done via batch. Internal methods, on the other hand, use references to original `Text` object to operate on them.\r\n\r\nYou can have multiple different `TextNode` connected with `Text` object. `TextNode` may be one or more characters from original `Text` object. `TextNode` has reference to original `Text` object (protected), `start` offset in original object (protected) and text that the `TextNode` is representing (public). Also it inherits everything from `Node` but `parent` property is readonly.\r\n\r\nNow that we touched `parent` property there is one dirty thing that I don't like but live with. As I said, `NodeList` keeps `Text` and they are referenced in `TextNode`s. But `Text` is not a node, so it does not have `parent`. I could go with two solutions:\r\n\r\n* introduce third class that would be basically `Text` with parent property and would be used only in `NodeList` and as a reference in `TextNode`, or\r\n* leave `Text` as is but add `parent` property when adding `Text` to a node.\r\n\r\nI've gone with the second option. If you think it is too dirty and \"silent\", I will introduce some kind of `NodeListText` class that will inherit from `Text` and add `parent` property.\r\n\r\nAnother solution that would solve some problems would be keeping `TextNode` in `NodeList` and keep reference to original `TextNode` in a `TextNode`."}}