TWC: Is it a good idea to save asynchronously?
175 views
Skip to first unread message
Pengju Yan
Jul 20, 2019, 11:14:55 AM7/20/19
to TiddlyWikiDev
Continue the title with "Or we need to modify TWC core substantially?"
I used savetiddlers (only version 0.6 worked on my FireFox with macOS while 0.8 simply didn't) to save my TWC until FireFox was automatically updated to 68.0.1 last week.
I learned all the code of savetiddlers and the saving mechanism of TWC while I was trying to fix some problems in savetiddlers. Although I'm not 100% sure, I'm suspicious about the asynchronous philosophy (as Yakov mentioned several times), at least for the current implementation of TWC core.
To my understanding, saveMain() in TWC will set the dirty flag to false after saveFile() returns. Consider you are working a big TWC file, and saving take several seconds to accomplish. Between subsequent 2 savings, something will happen this way:
1. You finish editing a tiddler A and click the done button. A save message will send to a saver.
2. Because the saving operation is asynchronous, saveFile() immediately returns and then the dirty flag is set to false.
3. The saver is downloading the new TWC file with the update tiddler A.
4. Before the new TWC file downloading finishes, you edit another tiddler B and finish editing it and click the done button. The TWC core will load the original TWC file from the local file system WITHOUT the updated tiddler A, and send another saving message to the saver. Because in step 2 the dirty flag is set to false, this time the updated content contains only updated tiddler B and the ORIGINAL tidder A (tiddler A is thought to be already up-to-date).
5. The saver makes a new download operation where the TWC file to be saved contains only the original content with the updated tiddler B.
6. A few seconds later, the first downloading finishes, now the local TWC file contains original content and updated tiddler A.
7. Another few seconds later, the second downloading finishes, now the local TWC file contains original content and updated tiddler B.
8. Note that ultimately you lose your work on tiddler A.
To be honest, I haven't done any test to check whether the situation above will happen. But anyway, I think asynchronous saving needs more thorough thinking.
To enable asynchronous loading and saving, I think we need to modify TWC core so that only if a "save-successful" message is received then the dirty flag could be set to false.
BJ
Jul 20, 2019, 2:57:38 PM7/20/19
to TiddlyWikiDev
there is a work around for using savetiddlers with ff 68 see
bj
Yakov
Jul 21, 2019, 4:54:24 PM7/21/19
to TiddlyWikiDev
Hi Pengju,
суббота, 20 июля 2019 г., 18:14:55 UTC+3 пользователь Pengju Yan написал:
суббота, 20 июля 2019 г., 18:14:55 UTC+3 пользователь Pengju Yan написал:
Continue the title with "Or we need to modify TWC core substantially?"
actually, I'm currently working on this and certain changes will be present in TW 2.9.3. I'm certain that async loading will make it into the next release (loadFile will try to load in an async fashion when it gets a second argument which should be a callback; and if XMLHttpRequest fails it will fallback to sync loading; also the async part will be in a separate method which can be overwritten by some event-driven loader or any other service).
Once async loading is implemented (note: loading is used by saving; actually, I've already implemented async loading, but I have to test it before committing), saving will work "faster" ("hanging" will be shorter) and also the road to async saving will be clear.. more or less: saving is more tricky because of download saving. Still, I think I have an idea how to approach the issue correctly.
When async saving is implemented, savers will be able to overwrite it as well and the IO functionality will be consistent (although some work on documenting the approach is also required so that the dev ecosystem becomes stable and there's as little need to distinguish TWC and TW5 as possible). However, for TWC there's one more issue which is only applies to upgrading the core. In my tests of upgrading when Timimi is used as a saver, I've figured that the script is injected too late when the upgrading already tried to save and failed. Though, that was true for the script in dev environment and things (like you've noticed) may be different for production extension and I should retest this next week with the Timimi released with my patch for TWC support.
I also have to notice that saving methods in TWC should be refactored in another aspect: they wrongly mix file manipulation and various reporting stuff which makes those difficult to tweak for certain purposes; but that's not that important for now.
Once async loading is implemented (note: loading is used by saving; actually, I've already implemented async loading, but I have to test it before committing), saving will work "faster" ("hanging" will be shorter) and also the road to async saving will be clear.. more or less: saving is more tricky because of download saving. Still, I think I have an idea how to approach the issue correctly.
When async saving is implemented, savers will be able to overwrite it as well and the IO functionality will be consistent (although some work on documenting the approach is also required so that the dev ecosystem becomes stable and there's as little need to distinguish TWC and TW5 as possible). However, for TWC there's one more issue which is only applies to upgrading the core. In my tests of upgrading when Timimi is used as a saver, I've figured that the script is injected too late when the upgrading already tried to save and failed. Though, that was true for the script in dev environment and things (like you've noticed) may be different for production extension and I should retest this next week with the Timimi released with my patch for TWC support.
I also have to notice that saving methods in TWC should be refactored in another aspect: they wrongly mix file manipulation and various reporting stuff which makes those difficult to tweak for certain purposes; but that's not that important for now.
I used savetiddlers (only version 0.6 worked on my FireFox with macOS while 0.8 simply didn't) to save my TWC until FireFox was automatically updated to 68.0.1 last week.I learned all the code of savetiddlers and the saving mechanism of TWC while I was trying to fix some problems in savetiddlers. Although I'm not 100% sure, I'm suspicious about the asynchronous philosophy (as Yakov mentioned several times), at least for the current implementation of TWC core.To my understanding, saveMain() in TWC will set the dirty flag to false after saveFile() returns. Consider you are working a big TWC file, and saving take several seconds to accomplish. Between subsequent 2 savings, something will happen this way:1. You finish editing a tiddler A and click the done button. A save message will send to a saver.2. Because the saving operation is asynchronous, saveFile() immediately returns and then the dirty flag is set to false.3. The saver is downloading the new TWC file with the update tiddler A.4. Before the new TWC file downloading finishes, you edit another tiddler B and finish editing it and click the done button. The TWC core will load the original TWC file from the local file system WITHOUT the updated tiddler A, and send another saving message to the saver. Because in step 2 the dirty flag is set to false, this time the updated content contains only updated tiddler B and the ORIGINAL tidder A (tiddler A is thought to be already up-to-date).
Is it how SaveTiddlers work? Because there's no such problem for TWC usual saving which is not tiddler-wise. Instead, it loads "original" (TW file), grabs the TW core and ignores the content completely and puts the current content into that core and than saves the file.
I've implemented tiddler-wise saving (I also call it "granulated") in MainTiddlyServer, but there's no such problem too because granulated saving doesn't load original at all: instead, it sends "changes" to the server and it updates TW (yeah, this requires the server to understand TW structure). MTS also supports non-granulated saving which is used for TWs below 2.8.1.
I've implemented tiddler-wise saving (I also call it "granulated") in MainTiddlyServer, but there's no such problem too because granulated saving doesn't load original at all: instead, it sends "changes" to the server and it updates TW (yeah, this requires the server to understand TW structure). MTS also supports non-granulated saving which is used for TWs below 2.8.1.
5. The saver makes a new download operation where the TWC file to be saved contains only the original content with the updated tiddler B.6. A few seconds later, the first downloading finishes, now the local TWC file contains original content and updated tiddler A.7. Another few seconds later, the second downloading finishes, now the local TWC file contains original content and updated tiddler B.8. Note that ultimately you lose your work on tiddler A.To be honest, I haven't done any test to check whether the situation above will happen. But anyway, I think asynchronous saving needs more thorough thinking.To enable asynchronous loading and saving, I think we need to modify TWC core so that only if a "save-successful" message is received then the dirty flag could be set to false.
See above
Pengju Yan
Jul 21, 2019, 10:57:44 PM7/21/19
to TiddlyWikiDev
On Monday, July 22, 2019 at 4:54:24 AM UTC+8, Yakov wrote:
Is it how SaveTiddlers work? Because there's no such problem for TWC usual saving which is not tiddler-wise. Instead, it loads "original" (TW file), grabs the TW core and ignores the content completely and puts the current content into that core and than saves the file.
My bad. Thank you for explaining to me how the TWC core works. I've quickly checked out the line "var posDiv = locateStoreArea(original);" and noticed that it really loads all the content but not only the "dirty" tidders. The original post was totally based on my misunderstanding of TWC core.
However, for TWC there's one more issue which is only applies to upgrading the core. In my tests of upgrading when Timimi is used as a saver, I've figured that the script is injected too late when the upgrading already tried to save and failed. Though, that was true for the script in dev environment and things (like you've noticed) may be different for production extension and I should retest this next week with the Timimi released with my patch for TWC support.
Cool, looking forward to the solution to the upgrading support. Other aspects (you may have already tested) we may need to pay attention to is to make sure the saver works for importing and exporting (I use ExportTiddlersPlugin for exporting, although it can't be found since http://www.tiddlytools.com/ is no longer accessible).
Yakov
Jul 22, 2019, 3:04:41 AM7/22/19
to TiddlyWikiDev
However, for TWC there's one more issue which is only applies to upgrading the core. In my tests of upgrading when Timimi is used as a saver, I've figured that the script is injected too late when the upgrading already tried to save and failed. Though, that was true for the script in dev environment and things (like you've noticed) may be different for production extension and I should retest this next week with the Timimi released with my patch for TWC support.Cool, looking forward to the solution to the upgrading support.
By the way, I'm not sure about other savers (browser extensions), but TiddlyFox supports upgrading already (I've fixed some issues of the core in 2.9.2); may be I'll implement support of upgrading via download saving in the next release(s) but yeah, it requires more tweaking for wide support.
Other aspects (you may have already tested) we may need to pay attention to is to make sure the saver works for importing and exporting (I use ExportTiddlersPlugin for exporting, although it can't be found since http://www.tiddlytools.com/ is no longer accessible).
Like many other repos, TiddlyTools is still accessible via web archive; I've tried to contact Eric to learn what happened via email but he hasn't replied. Are there any problems with ExportTiddlersPlugin? Import should work fine without any saver since it uses standart web API. Also, you may want to check out TiddlerInFilePlugin and SharedTiddlersPlugin. Basically the support of export may be extended to supporting download saving; in other aspects it's the same as support of saving arbitrary [text] files which depends on a saver engine. Some forms of "importing" (like those in TIFP and STP) in certain use-cases require support of loading files automatically from outside of current folder which normally relies on browser support (xhr restrictions is governed by the
security.fileuri.strict_origin_policy setting in FireFox and Chrome can be launched with the --allow-file-access-from-files setting although I haven't tested it for a long time) – that's an ok approach but ideally we need a "saver" which proxies loading and makes it stable (well, MainTiddlyServer implements that partially; perhaps I'll propose that for Timimi too but for now I don't know how to setup and test it).BJ
Jul 22, 2019, 4:37:52 AM7/22/19
to TiddlyWikiDev
Hi Yakov,
savetiddlers overrides the functions mozillaLoadFile() and mozillaSaveFile() the same as tiddlyfox. With chrome and now with firefox (since version 68) loading files from 'file:/' is blocked due to security concerns, this means that when saving the function recreateOriginal() is used to create the new twc image. There is a problem with this function, in that it uses what is in the dom for the new image, this will include the message box div that has been injected by savetiddlers. It is possible that other browser plugins will inject in the dom as well. It would be better to remove the message box from the new image. At present savetiddlers will not work with a twc that already has a message box div - I will work around this and produce a new release.
cheers
BJ
Pengju Yan
Jul 22, 2019, 9:43:17 PM7/22/19
to tiddly...@googlegroups.com
On Saturday, July 20, 2019 at 11:14:55 PM UTC+8, Pengju Yan wrote:
Continue the title with "Or we need to modify TWC core substantially?"
To enable asynchronous loading and saving, I think we need to modify TWC core so that only if a "save-successful" message is received then the dirty flag could be set to false.
Also, the dirty flags of successive savings should be managed in a stack and cleaned in a proper way (delete certain dirty flags upon receiving certain "save-successful" messages).
Pengju Yan
Jul 22, 2019, 9:55:20 PM7/22/19
to tiddly...@googlegroups.com
A really serious problem in savetiddlers is the mechanism of avoiding saving the same file while a previous saving is not finished. Check the dict "debouncing" in "contentscript.js".
In several cases, the dict "debouncing" was not cleared as expected then all my subsequent work was lost without my awareness (I enabled autosave and savebackups). I've been suffering from this time by time.
So this is my suggestion: Don't use savetiddlers any more.
Yakov
Jul 23, 2019, 3:15:44 AM7/23/19
to TiddlyWikiDev
Hi BJ,
when saving the function recreateOriginal() is used to create the new twc image
yeah, yesterday I figured too that its the source of the problem. I use a plugin which overwrites loadOriginal to remove using recreateOriginal because TiddlyFox wrongly always reports successful saving and I have some TWs on an ejectable USB storage, so I need to be sure that if saving failed TW reports fail (TW may be still opened but the storage may be ejected). That's why I overlooked the issue.
With chrome and now with firefox (since version 68) loading files from 'file:/' is blocked due to security concerns
That's not quite true, setting
security.fileuri.strict_origin_policy in FF still makes it possible to load files via xhr through file: schema (I haven't tested much, but launching Chrome with the --allow-file-access-from-files param should do so as well).It would be better to remove the message box from the new image. At present savetiddlers will not work with a twc that already has a message box div - I will work around this and produce a new release.
Could you provide some more details on why it doesn't work now? Because I'm considering even adding the message box to the core so that development of future savers that use the event-driven saving is easier.
Pengju Yan wrote
A really serious problem in savetiddlers is the mechanism of avoiding saving the same file while a previous saving is not finished. Check the dict "debouncing" in "contentscript.js".
In several cases, the dict "debouncing" was not cleared as expected then all my subsequent work was lost without my awareness (I enabled autosave and savebackups). I've been suffering from this time by time.
Oh gosh, do you mean you had 2 subsequent saves with a little time gap between them? (could you describe your scenario so that I'm more aware of this potential problem? MainTiddlySaver hasn't debouncing implemented yet, but I never experienced this problem with content, only with options storage) If there's such problem, we should describe it in detail and refer to this in future development; I wasn't expecting anything like this in an extension saver (or may be the source of the problem is a bit different from what you describe?) We really need a reproducible scenario.
Also, the dirty flags of successive savings should be managed in a stack and cleaned in a proper way (delete certain dirty flags upon receiving certain "save-successful" messages).
Yeah, I second this proposal (thinking about it for some time already), but this is a second level of IO improving which I'd address when a) I finish making TWC saving async and b) some documentation about IO/developing savers and editing (collaborating) workflow/infrastructure is established – because this is a more complicated idea and is less trivial to implement, especially when there's no docs describing it.
Best regards,
Yakov.
вторник, 23 июля 2019 г., 4:43:17 UTC+3 пользователь Pengju Yan написал:
Best regards,
Yakov.
вторник, 23 июля 2019 г., 4:43:17 UTC+3 пользователь Pengju Yan написал:
BJ
Jul 23, 2019, 9:26:17 AM7/23/19
to TiddlyWikiDev
Hi Yakov,
savetiddlers assumes that if there is a message box then another saver has added it and therefore savetiddlers will deactive as it is not needed. I will change this to check to the attribute "data-message-box-creator" having value "savetiddlers". This means you can add the message box to the core twc, just make the "data-message-box-creator" an empty string.
The main problem I see with the saving is the twc assume that saving is synchronous when it is in fact asynchronous. Tiddlyfox and savetiddler fire the event "tiddlyfox-have-saved-file" when the tiddlywiki is saved, - in tw5 this is listened for and only when it fires is the message displayed that says the tw is saved.
Pengju Yan
Jul 24, 2019, 12:37:05 AM7/24/19
to TiddlyWikiDev
On Tuesday, July 23, 2019 at 3:15:44 PM UTC+8, Yakov wrote:
Oh gosh, do you mean you had 2 subsequent saves with a little time gap between them? (could you describe your scenario so that I'm more aware of this potential problem? MainTiddlySaver hasn't debouncing implemented yet, but I never experienced this problem with content, only with options storage) If there's such problem, we should describe it in detail and refer to this in future development; I wasn't expecting anything like this in an extension saver (or may be the source of the problem is a bit different from what you describe?) We really need a reproducible scenario.
Yes, exactly. I enabled both autosave and savebackups so that I don't need to worry about any minor mistaken typings (you know we don't have a REDO right?). My TWC file is of >6M size so saving takes seconds to finish. savetidllers work like this way: When the user triggers a saving, it puts file name in the "debouncing" dict and will remove it from the dict once the saving finishes. But in several cases, it just doesn't removed my file name from the "debouncing" dict and then all my subsequent savings will just don't happen. I guess this problem will happen if I trigger successive savings with just a very short time period in between, although I'm not very sure.
After several cases of catastrophic losses, I wrote a local crontab (macOS launchctl) script to monitor my backup folders to check whether subsequent backup files are of the same length (indicating that the main TWC file is not successfully changed). That's how I survived before Timimi+TWC appeared.
Seems like BJ is the developer of savetiddlers? I suggest the author takes a serious look at this issue (if the author likes to maintain it in a long term) and I'll help to discuss about the reproduction.
Anyway, I don't think "debouncing" is necessary. It does not hurt even if the user saves too frequently. So in my own (unsuccessful) modified savetiddlers, I simply removed this mechanism (Though it doesn't not work under FireFox 68.0.1).
I don't think we need any documentation talking about this problem, because it happens just within a specific saver.
Yeah, I second this proposal (thinking about it for some time already), but this is a second level of IO improving which I'd address when a) I finish making TWC saving async and b) some documentation about IO/developing savers and editing (collaborating) workflow/infrastructure is established – because this is a more complicated idea and is less trivial to implement, especially when there's no docs describing it.
Great. We can first make asynchronous saving work and then improves it.
BJ
Jul 24, 2019, 5:54:04 AM7/24/19
to TiddlyWikiDev
HI Pengju,
savetiddlers uses the download mechanism. With firefox on linux if 2 downloads to the same file are attempted (that is another download to the same file is started before the first has finished) both will fail and the original file will have be lost, this is why the de-bounce was added. I do not have access to a mac so cannot test on the mac. I understand that you are using version 0.6. I cannot try and fix older version of the extension. If you can tell me the problem you have with current version I will investigate.
cheers
BJ
BJ
Aug 6, 2019, 6:09:47 AM8/6/19
to TiddlyWikiDev
I have made a new version of savetiddlers for classic tw to work with the lastest firefox - I would appreciate it if you (or others) could try it.
thanks
bj
BJ
Aug 15, 2019, 11:15:15 AM8/15/19
to TiddlyWikiDev
Yakov
Aug 28, 2019, 4:37:51 PM8/28/19
to TiddlyWikiDev
Hi BJ,
thanks for both the update/release and the TiddlyTools link. I'll update the latter on classic.tiddlywiki.com; I can't predict how soon I'll try SaveTiddlers by itself, but the codebase is quite valuable since it covers FF and Chrome. Did I get the readme.md right that TW5 is saved in both Chrome and FireFox while for TWC only FF is supported?
Best regards,
Yakov.
вторник, 6 августа 2019 г., 13:09:47 UTC+3 пользователь BJ написал:
thanks for both the update/release and the TiddlyTools link. I'll update the latter on classic.tiddlywiki.com; I can't predict how soon I'll try SaveTiddlers by itself, but the codebase is quite valuable since it covers FF and Chrome. Did I get the readme.md right that TW5 is saved in both Chrome and FireFox while for TWC only FF is supported?
Best regards,
Yakov.
вторник, 6 августа 2019 г., 13:09:47 UTC+3 пользователь BJ написал:
BJ
Aug 29, 2019, 1:19:10 PM8/29/19
to tiddly...@googlegroups.com
with the latest code both twc and tw5 are supported on ff and chrome, but only versions of twc that support the html5 download mechanism (I think a html5 twc plugin could be written for olderversion).
cheers
BJ
--
You received this message because you are subscribed to a topic in the Google Groups "TiddlyWikiDev" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/tiddlywikidev/0XbHgZks9OM/unsubscribe.
To unsubscribe from this group and all its topics, send an email to tiddlywikide...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/tiddlywikidev/514de572-6502-48f6-9594-25ff4eada8cf%40googlegroups.com.
Reply all
Reply to author
Forward
0 new messages