[xwiki-devs] [VOTE] Introduce a cache of documents used in the current request
Hi devs, Currently the getDocument method always goes to the storage to retrieve the document, even if the same document has just been retrieved. This means that the following code will not work: #set($d = $xwiki.getDocument('X')) $d.setTitle('the title') $xwiki.getDocument('X').getTitle() # will not print 'the title' I'd like to change getDocument so that it first searches in a map of used documents in the current context. This means the following: - getDocument searches in XWikiContext.usedDocuments (or better, in the ExecutionContext) - if found, return the value from there - if not, go to the storage, return it to the caller - when the document is changed for the first time, i.e. when api.Document.getDoc() is called, clone the original document and put it in usedDocuments - as a special case, PreviewAction also puts the updated context document in usedDocuments This means that consecutive calls for retrieving a (changed) document will always return the same object. This prevents possible preview bugs, like http://jira.xwiki.org/jira/browse/XABLOG-14 or http://jira.xwiki.org/jira/browse/XWIKI-4689 Yet this is an important behavior change. Do you think anybody is using this "feature", and actually expects the above code example to work as it does now? Also, we must be careful with the performance, since this new map could get big, holding all the documents in the database. Perhaps a LRU fixed-size map would be better, although this breaks the uniqueness guarantee. So, WDYT? 1. Should we introduce this cache? 2. Should it be limited in size? -- Sergiu Dumitriu http://purl.org/net/sergiu/
On Dec 21, 2009, at 1:34 PM, Sergiu Dumitriu wrote:
Hi devs,
Currently the getDocument method always goes to the storage to retrieve the document, even if the same document has just been retrieved. This means that the following code will not work:
#set($d = $xwiki.getDocument('X')) $d.setTitle('the title') $xwiki.getDocument('X').getTitle() # will not print 'the title'
+1 in general for the idea. All unsaved changes are linked to the Session (known as the Context for us). For ex in JCR, you even have a Session.save() method that saves all unsaved changes in the workspace. And the methods to retrieve a document are done on the object graph itself or from the Session (getItem()), thus ensuring that modified objects are returned. Thus for the new Model, I agree we need to saved the list of modified Model objects in our Session (Execution Context). We might also need to add a save() method in the Model/ModelContext interface. Thanks -Vincent
I'd like to change getDocument so that it first searches in a map of used documents in the current context. This means the following: - getDocument searches in XWikiContext.usedDocuments (or better, in the ExecutionContext) - if found, return the value from there - if not, go to the storage, return it to the caller - when the document is changed for the first time, i.e. when api.Document.getDoc() is called, clone the original document and put it in usedDocuments - as a special case, PreviewAction also puts the updated context document in usedDocuments
This means that consecutive calls for retrieving a (changed) document will always return the same object. This prevents possible preview bugs, like http://jira.xwiki.org/jira/browse/XABLOG-14 or http://jira.xwiki.org/jira/browse/XWIKI-4689
Yet this is an important behavior change. Do you think anybody is using this "feature", and actually expects the above code example to work as it does now?
Also, we must be careful with the performance, since this new map could get big, holding all the documents in the database. Perhaps a LRU fixed-size map would be better, although this breaks the uniqueness guarantee.
So, WDYT?
1. Should we introduce this cache? 2. Should it be limited in size?
-- Sergiu Dumitriu http://purl.org/net/sergiu/
On Mon, Dec 21, 2009 at 13:34, Sergiu Dumitriu <[email protected]> wrote:
Hi devs,
Currently the getDocument method always goes to the storage to retrieve the document, even if the same document has just been retrieved. This
Note to be more clear: in public API only not private one (because the XWikiDocument is got from cache storage and not reloaded each time from the database). And the exact issue is that the XWikiDocument is cloned in the Document instance so when you ask again for the Document you don't get the same instance of XWikiDocument behind the scene.
means that the following code will not work:
#set($d = $xwiki.getDocument('X')) $d.setTitle('the title') $xwiki.getDocument('X').getTitle() # will not print 'the title'
I'd like to change getDocument so that it first searches in a map of used documents in the current context. This means the following: - getDocument searches in XWikiContext.usedDocuments (or better, in the ExecutionContext)
Well Document class is old API so not sure it's if really better in ExecutionContext
- if found, return the value from there - if not, go to the storage, return it to the caller - when the document is changed for the first time, i.e. when api.Document.getDoc() is called, clone the original document and put it in usedDocuments - as a special case, PreviewAction also puts the updated context document in usedDocuments
This means that consecutive calls for retrieving a (changed) document will always return the same object. This prevents possible preview bugs, like http://jira.xwiki.org/jira/browse/XABLOG-14 or http://jira.xwiki.org/jira/browse/XWIKI-4689
Yet this is an important behavior change. Do you think anybody is using this "feature", and actually expects the above code example to work as it does now?
Also, we must be careful with the performance, since this new map could get big, holding all the documents in the database. Perhaps a LRU fixed-size map would be better, although this breaks the uniqueness guarantee.
So, WDYT?
1. Should we introduce this cache?
+1 for a context cache for public Document instances, makes the scripts more natural
2. Should it be limited in size?
Always better I think
-- Sergiu Dumitriu http://purl.org/net/sergiu/ _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
Hi, On Mon, Dec 21, 2009 at 6:15 PM, Thomas Mortagne <[email protected]>wrote:
On Mon, Dec 21, 2009 at 13:34, Sergiu Dumitriu <[email protected]> wrote:
Hi devs,
Currently the getDocument method always goes to the storage to retrieve the document, even if the same document has just been retrieved. This
Note to be more clear: in public API only not private one (because the XWikiDocument is got from cache storage and not reloaded each time from the database). And the exact issue is that the XWikiDocument is cloned in the Document instance so when you ask again for the Document you don't get the same instance of XWikiDocument behind the scene.
So with the new changes we'd clone the Document instance in current execution context (if there is one) rather than retrieve the underlying XWikiDocument and create a new Documents instance? +1 for the idea. - Asiri
participants (4)
-
Asiri Rathnayake -
Sergiu Dumitriu -
Thomas Mortagne -
Vincent Massol