Re: [fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
Inoussa OUEDRAOGO wrote: 2008/8/6 Micha Nelissen [EMAIL PROTECTED]: Perhaps another option is to remove the NodeMemManager altogether and simply use GetMem/FreeMem (or New/Dispose)? The second proposition contains a default TAVLTree that remove the node mem manager and TAVLManagedTree that has a node mem manager for people who do need it that way. Was that (a) or (b)? I didn't see this? Micha ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
Am Mittwoch, 6. August 2008 21:37 schrieb Sergei Gorelkin: Mattias Gaertner wrote: On Wed, 6 Aug 2008 19:41:27 +0100 Inoussa OUEDRAOGO [EMAIL PROTECTED] wrote: Hi, TAVLTree in avl_tree.pp is not thread safe due to the node allocation and de-allocation done through the global declared NodeMemManager variable. TAVLTreeNodeMemManager implementation is cleary not thread safe, which btw IMHO is a good thing ( for performance reason). Proposition : (a) TAVLTree should allow, at construct time, to specify a Node memory manager which will be kept and used. If not specified the global one will be used. (b) NodeMemManager should be declared as ThreadVar. This changes does not break the API while making the implementation thread safe. By the way note that the XML DOM's implementation ( TDOMNode_WithChildren ) uses TAVLTree, so every code that uses the fcl-xml package mainly the DOM unit, is not thread safe. If this proposition is accepted it will be nice to have it introduced in the soon to be release fpc 2.2.2, in the case that is still possible, mainly for server programming. Attached is a patch that demonstrate the above proposition. Providing a local node mem manager does not repair xml dom. Maybe move NodeMemManager to the interface, so that a user can replace it with a threadsafe one? Since this topic is touched, I would like to vote for removing avl_tree dependency from the DOM altogether. The reason is that the avl tree of child nodes is useless for any real-world XML data because it does not handle duplicate names. The tree is useful only for the one particular case of XML configs where each node name is unique. OTOH, it causes considerable performance penalties (each insert results in two searches, first one for checking the existence, second one for insert itself; each search is a number of calls to Node.GetNodeName, which, in case of Windows, is a Widestring copy, etc.) Therefore I'd suggest to remove the avltree from DOM. This would be great to have in 2.2.2. In order to keep configs at the reasonable speed, it is possible to implement indexing within TXMLConfig class itself, preferably hashmap-based. This would be a performance tweak which (from my POV) does not need to be in 2.2.2, although it would be nice. Alternatively, it is possible to use Registry-like access (OpenKey/CloseKey methods) that I had already implemented in newer xmlconf.pp unit. The advantage is that the searches are done starting from the 'opened key', not from the root, so linear search isn't too slow. If I understand correctly, this would change the usage of xmlconfig which would require changes to any existing code using it? If so, it's probably not an option for 2.2.2, maybe even not for 2.2.4 .. regards Burkhard ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
Zitat von Burkhard Carstens [EMAIL PROTECTED]: Am Mittwoch, 6. August 2008 21:37 schrieb Sergei Gorelkin: Since this topic is touched, I would like to vote for removing avl_tree dependency from the DOM altogether. The reason is that the avl tree of child nodes is useless for any real-world XML data because it does not handle duplicate names. The tree is useful only for the one particular case of XML configs where each node name is unique. OTOH, it causes considerable performance penalties (each insert results in two searches, first one for checking the existence, second one for insert itself; each search is a number of calls to Node.GetNodeName, which, in case of Windows, is a Widestring copy, etc.) The widestrings version is slow under linux too. Last time I tested (a few months ago) it was about 2 to 10 times slower than the ansistring version. Mainly because of the parser. Therefore I'd suggest to remove the avltree from DOM. This would be great to have in 2.2.2. In order to keep configs at the reasonable speed, it is possible to implement indexing within TXMLConfig class itself, preferably hashmap-based. This would be a performance tweak which (from my POV) does not need to be in 2.2.2, although it would be nice. The performance penalty without the trees can be quite big for things like txmlconfig. For example for lazarus project files it can be a hundred times slower without the trees. Note: lazarus itself uses its own ansistring version of xml parser, so this not a direct problem for lazarus. Alternatively, it is possible to use Registry-like access (OpenKey/CloseKey methods) that I had already implemented in newer xmlconf.pp unit. The advantage is that the searches are done starting from the 'opened key', not from the root, so linear search isn't too slow. If I understand correctly, this would change the usage of xmlconfig which would require changes to any existing code using it? If so, it's probably not an option for 2.2.2, maybe even not for 2.2.4 .. I second that. The main advantage of TXMLConfig is it's easy usage. I guess you can gain some speed by implementing some kind of caching for paths. Maybe I will do that for the lazarus ansistring version. But IMO this is too risky for fpc 2.2.2 which is released soon. About avl tree: For thread safety I guess the best thing is to not use the node mem manager. Mattias ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
Am Mittwoch, 6. August 2008 20:41 schrieb Inoussa OUEDRAOGO: Hi, TAVLTree in avl_tree.pp is not thread safe due to the node allocation and de-allocation done through the global declared NodeMemManager variable. TAVLTreeNodeMemManager implementation is cleary not thread safe, which btw IMHO is a good thing ( for performance reason). Proposition : (a) TAVLTree should allow, at construct time, to specify a Node memory manager which will be kept and used. If not specified the global one will be used. (b) NodeMemManager should be declared as ThreadVar. This changes does not break the API while making the implementation thread safe. By the way note that the XML DOM's implementation ( TDOMNode_WithChildren ) uses TAVLTree, so every code that uses the fcl-xml package mainly the DOM unit, is not thread safe. If this proposition is accepted it will be nice to have it introduced in the soon to be release fpc 2.2.2, in the case that is still possible, mainly for server programming. Attached is a patch that demonstrate the above proposition. What about this: * let TAVLTree access the node manager using a local reference (TAVLTree.FNodeMemManager) * in TAVLTree.create: if IsMultiThread then FNodeMemManager:=TAVLTreeNodeMemManager.create else FNodeMemManager:=NodeMemManager; * in TAVLTree.destroy: if IsMultiThread then FNodeMemManager.free; This way, nothing would change for single threaded apps but it would be fully threadsafe for multithreaded (AFAIKS). This is probably not a perfect solution, but the least intrusive one .. regards Burkhard ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
Am Mittwoch, 6. August 2008 20:41 schrieb Inoussa OUEDRAOGO: What about this: * let TAVLTree access the node manager using a local reference (TAVLTree.FNodeMemManager) * in TAVLTree.create: if IsMultiThread then FNodeMemManager:=TAVLTreeNodeMemManager.create else FNodeMemManager:=NodeMemManager; * in TAVLTree.destroy: if IsMultiThread then FNodeMemManager.free; This way, nothing would change for single threaded apps but it would be fully threadsafe for multithreaded (AFAIKS). This is probably not a perfect solution, but the least intrusive one .. I would add nodemgr as parameter to create, but with a default value of nil. If not NIL then just grab the default nodemanager. That way, people with multithreading concerns can override, and it is perfectly backwards compatible. There is no good system to do this kind of stuff automatic: - I might have threads but only read a xml config in the mainthread, no need for locks - I might have a dozen xml readers somewhere, and they all get an own pool of nodes, inflating memory. _IF_ it is multithread. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
[fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
Hi, TAVLTree in avl_tree.pp is not thread safe due to the node allocation and de-allocation done through the global declared NodeMemManager variable. TAVLTreeNodeMemManager implementation is cleary not thread safe, which btw IMHO is a good thing ( for performance reason). Proposition : (a) TAVLTree should allow, at construct time, to specify a Node memory manager which will be kept and used. If not specified the global one will be used. (b) NodeMemManager should be declared as ThreadVar. This changes does not break the API while making the implementation thread safe. By the way note that the XML DOM's implementation ( TDOMNode_WithChildren ) uses TAVLTree, so every code that uses the fcl-xml package mainly the DOM unit, is not thread safe. If this proposition is accepted it will be nice to have it introduced in the soon to be release fpc 2.2.2, in the case that is still possible, mainly for server programming. Attached is a patch that demonstrate the above proposition. Best regards. -- Inoussa O. avl_tree.dif Description: Binary data ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
On Wed, 6 Aug 2008 19:41:27 +0100 Inoussa OUEDRAOGO [EMAIL PROTECTED] wrote: Hi, TAVLTree in avl_tree.pp is not thread safe due to the node allocation and de-allocation done through the global declared NodeMemManager variable. TAVLTreeNodeMemManager implementation is cleary not thread safe, which btw IMHO is a good thing ( for performance reason). Proposition : (a) TAVLTree should allow, at construct time, to specify a Node memory manager which will be kept and used. If not specified the global one will be used. (b) NodeMemManager should be declared as ThreadVar. This changes does not break the API while making the implementation thread safe. By the way note that the XML DOM's implementation ( TDOMNode_WithChildren ) uses TAVLTree, so every code that uses the fcl-xml package mainly the DOM unit, is not thread safe. If this proposition is accepted it will be nice to have it introduced in the soon to be release fpc 2.2.2, in the case that is still possible, mainly for server programming. Attached is a patch that demonstrate the above proposition. Providing a local node mem manager does not repair xml dom. Maybe move NodeMemManager to the interface, so that a user can replace it with a threadsafe one? Mattias ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
Inoussa OUEDRAOGO wrote: TAVLTree in avl_tree.pp is not thread safe due to the node allocation and de-allocation done through the global declared NodeMemManager variable. TAVLTreeNodeMemManager implementation is cleary not thread safe, which btw IMHO is a good thing ( for performance reason). Proposition : (a) TAVLTree should allow, at construct time, to specify a Node memory manager which will be kept and used. If not specified the global one will be used. (b) NodeMemManager should be declared as ThreadVar. Huh? What's the point of having a global (read: constant) default declared as threadvar? So, (b) is unnecessary (at least in your patch). Another solution would be to declare it as ThreadVar and overwriting it upon construction. But wait... this makes me think if we can't do it automatically: If we have a new thread, we need a new instance in most cases anyway (except when there's actually only one thread handling XML-data, which I consider rather unlikely). This changes does not break the API while making the implementation thread safe. Depending solely on the discipline of the caller. And consider the XML-case, it wouldn't even be possible without changing this API, too. And most likely a lot of other APIs on the way to tackle it down. By the way note that the XML DOM's implementation ( TDOMNode_WithChildren ) uses TAVLTree, so every code that uses the fcl-xml package mainly the DOM unit, is not thread safe. If this proposition is accepted it will be nice to have it introduced in the soon to be release fpc 2.2.2, in the case that is still possible, mainly for server programming. As I just pointed out, it wouldn't make handling XML data threadsafe. There's no way to specify upon creation of the TXMLDocument that this will be used in another thread, needing a new NodeManager instance. Vinzent. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
Mattias Gaertner wrote: On Wed, 6 Aug 2008 19:41:27 +0100 Inoussa OUEDRAOGO [EMAIL PROTECTED] wrote: Hi, TAVLTree in avl_tree.pp is not thread safe due to the node allocation and de-allocation done through the global declared NodeMemManager variable. TAVLTreeNodeMemManager implementation is cleary not thread safe, which btw IMHO is a good thing ( for performance reason). Proposition : (a) TAVLTree should allow, at construct time, to specify a Node memory manager which will be kept and used. If not specified the global one will be used. (b) NodeMemManager should be declared as ThreadVar. This changes does not break the API while making the implementation thread safe. By the way note that the XML DOM's implementation ( TDOMNode_WithChildren ) uses TAVLTree, so every code that uses the fcl-xml package mainly the DOM unit, is not thread safe. If this proposition is accepted it will be nice to have it introduced in the soon to be release fpc 2.2.2, in the case that is still possible, mainly for server programming. Attached is a patch that demonstrate the above proposition. Providing a local node mem manager does not repair xml dom. Maybe move NodeMemManager to the interface, so that a user can replace it with a threadsafe one? Since this topic is touched, I would like to vote for removing avl_tree dependency from the DOM altogether. The reason is that the avl tree of child nodes is useless for any real-world XML data because it does not handle duplicate names. The tree is useful only for the one particular case of XML configs where each node name is unique. OTOH, it causes considerable performance penalties (each insert results in two searches, first one for checking the existence, second one for insert itself; each search is a number of calls to Node.GetNodeName, which, in case of Windows, is a Widestring copy, etc.) Therefore I'd suggest to remove the avltree from DOM. In order to keep configs at the reasonable speed, it is possible to implement indexing within TXMLConfig class itself, preferably hashmap-based. Alternatively, it is possible to use Registry-like access (OpenKey/CloseKey methods) that I had already implemented in newer xmlconf.pp unit. The advantage is that the searches are done starting from the 'opened key', not from the root, so linear search isn't too slow. Regards, Sergei ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
Re: [fpc-devel] TAVLTree(avl_tree.pp) thread safety, fcl-xml(DOM) is also concerned.
2008/8/6 Micha Nelissen [EMAIL PROTECTED]: Inoussa OUEDRAOGO wrote: Hi, TAVLTree in avl_tree.pp is not thread safe due to the node allocation and de-allocation done through the global declared NodeMemManager variable. TAVLTreeNodeMemManager implementation is cleary not thread safe, which btw IMHO is a good thing ( for performance reason). Perhaps another option is to remove the NodeMemManager altogether and simply use GetMem/FreeMem (or New/Dispose)? The second proposition contains a default TAVLTree that remove the node mem manager and TAVLManagedTree that has a node mem manager for people who do need it that way. -- Inoussa O. ___ fpc-devel maillist - fpc-devel@lists.freepascal.org http://lists.freepascal.org/mailman/listinfo/fpc-devel
