View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001793 | JEDI VCL | 00 JVCL Components | public | 2004-05-24 14:34 | 2004-05-28 01:07 |
| Reporter | bgn | Assigned To | user72 | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | |||||
| Target Version | Fixed in Version | ||||
| Summary | 0001793: XmlAppstorage and Docking storage problem | ||||
| Description | If you use xmlappstorage and use it with LoadDockTreeFromAppStorage <FormNames tag increases in length each run (gets quite slow). | ||||
| Additional Information | AppIniFileStorage works. Compiled with the Delphi 7 and the latest source. | ||||
| Tags | No tags attached. | ||||
|
2004-05-24 14:34
|
DockingLayout.zip (5,367 bytes) |
|
|
> FormNames tag increases in length each run FormNames does not increase in length: it contains one entry for each dock area (top, bottom, left and right) + one netry for the main form. The slowness is caused by the AppStorage having AutoFlush and AutoReload set to true. Set them both to false and change your code to: procedure TForm1.FormCreate(Sender: TObject); begin Form2 := TForm2.Create(nil); //LoadDockTreeFromAppStorage(JvAppIniFileStorage1); JvAppXMLFileStorage1.Reload; LoadDockTreeFromAppStorage(JvAppXMLFileStorage1); Form2.Show; end; procedure TForm1.FormDestroy(Sender: TObject); begin // SaveDockTreeToAppStorage(JvAppIniFileStorage1); SaveDockTreeToAppStorage(JvAppXMLFileStorage1); JvAppXMLFileStorage1.Flush; end; Setting AutoReload and AutoFlush to true will reload and save the file each time you call any of the ReadXXX and WriteXXX functions of the AppStorage and they are called a lot by LoadDockTreeFromAppStorage and SaveDockTreeToAppStorage. |
|
|
The first thing that would pop in my head is: why not (temporarily) set these properties to False inside the Load/SaveDockTreeFromAppStorage functions and restoring them when done. But IIRC, you can call BeginUpdate/EndUpdate or something like that on the AppStorages, so that would make it even easier (there are other generic Load/Save routines using AppStorage that will benefit from this as well). |
|
|
I had the same thought and I'll look into changing it in at least the docking support. A better solution would be to find a way to support AutoReload and AutoFlush without constantly reloading and saving on each read and write but I haven't figured out anything clever yet... |
|
|
>>> A better solution would be to find a way to support AutoReload and AutoFlush without constantly reloading and saving on each read and write but I haven't figured out anything clever yet... <<< Register a change notification on the file? |
|
|
> Register a change notification on the file? That's sounds interesting. Care to elaborate? |
|
|
Well, my knowledge what is possible with ChangeNotification is somewhat limited, but as I understand it you can register a callback to be executed when a change takes place in the file system. If we can do that for changes in the file currently in use by the AppStore, we can perform the Reload at that point (if AutoReload = True of course and the change did not occur by the app storage itself). Only the AutoFlush is not going to work that way. Originally I had implemented an idle handler to flush when the application was actually idle for at least x seconds, but that was not working properly and lead to a number of app. shutdown problems. Ideally, something like that should be implemented (possibly the the delay part should be left out and just the OnIdle handling part should return; this would remove the thread and the shutdown problems). |
|
|
I tried your suggestion and diabled the auto flush/reload feature. And it is much faster. But... the <FormNames> tag is still expanding for each run of the test program. After two runs i get the following... <FormNames>Form2 Form1@TopDockPanel_A_B_C_D_E_F_G Form1@BottomDockPanel_A_B_C_D_E_F_G Form1@LeftDockPanel_A_B_C_D_E_F_G Form1@RightDockPanel_A_B_C_D_E_F_G Form1 Form2;Form1@TopDockPanel_A_B_C_D_E_F_G;Form1@BottomDockPanel_A_B_C_D_E_F_G;Form1@LeftDockPanel_A_B_C_D_E_F_G;Form1@RightDockPanel_A_B_C_D_E_F_G;Form1;</FormNames> the blanks above are really '\0'. And it the fails to restore the docking info. |
|
|
> But... the <FormNames> tag is still expanding for each run of the test program. That is very strange. I tried it with your demo project and ran it several times without that happening to me. What version of the dock files/app storages do you have (the version is at the top of each unit, just below the license info). Are you sure that the units in your \run folder is the ones actually used at run-time (i.e do you have more than one copy of the jvcl units/dcus)? Here's my list (not all apply to this issue): JvAppStorage.pas 1.35 JvAppIniStorage.pas 1.23 JvAppXMLStorage.pas 1.19 JvDockControlForm.pas 1.36 JvDockGlobals.pas 1.18 JvDockInfo.pas 1.19 JvDockSupportClass.pas 1.7 JvDockSupportControl.pas 1.13 JvDockSupportProc.pas 1.6 JvDockTree.pas 1.12 |
|
|
> my knowledge what is possible with ChangeNotification [...] ChangeNotifications can only be used on folders, not files so using it could prove more complicated than necessary. I actually implemented a FileNotify thread class (it checks the modify date of the file once every second or so) in another project I work on (IniTranslator) but it has some of the same problems you mentioned (sometimes it thinks an internal modify is an external modify and vice versa - very random and doesn't happen that often). A more "clever" approach without using threads or notifications would be nice. What I mean is some kind of logic that can determine if a Reload or flush is actually needed. It could be as simple as setting an internal variable (Reload -> FReloaded , Flush -> FFlushed and only actually call Reload if FReloaded is false etc) but I don't have enough knowledge about how the AppStorage subsystem actually works in real-time to come up with something I like. |
|
|
>>> What I mean is some kind of logic that can determine if a Reload or flush is actually needed. It could be as simple as setting an internal variable (Reload -> FReloaded , Flush -> FFlushed and only actually call Reload if FReloaded is false etc) but I don't have enough knowledge about how the AppStorage subsystem actually works in real-time to come up with something I like. <<< The Reload is to handle cases where the backing file (XML or INI) will be reread into the internal buffer (since all file storages are buffered) before a change is written to it. This will prevent values to disappear if the file is accessed from outside of the AppStorage. Flush is used to write the internal (and possibly changed) buffer to the file so that when it is used from outside the AppStorage, it will not miss the changes/additions/removals that have taken place. Personally, I would usually set the AutoFlush/AutoReload to False anyway, since I will be using the AppStorage to access the data at all times. As I mentioned in some other report (forgot which) we can always provide a TCustomIniFile descendant that will reroute to the AppStorage if users like to continue using the current TIniFile code (would only require them to change the creation of the TCustomIniFile instance, instead of changing all calls to use the AppStorage), but I don't think it's actually neccessary. |
|
|
Here is the file that i get. I've taken the latest source from cvs for jcl/jvcl and removed and installed it again. And it still does the same. I've been using the 1.91 jcl (do I need to use the 1.90 branch ?) I have the same files as you except: >JvAppStorage.pas 1.35 In cvs there is only the 1.34 version. Which isn't in cvs ? |
|
|
> Which isn't in cvs ? They are all in CVS but anon access can be delayed (from a couple of hours to a couple of days), so maybe you should try doing a download tomorrow and try again. Could someone else try the demo so we can determine if this (ever expanding <FormNames>) is indeed a bug? |
|
|
I've updated with the latest changes (1.35 of appStorage etc) and no change. |
|
|
Got it! It's the XML storage that's acting up here (I was looking at the ini storage...). Here's a solution but I don't know if this will work in all cases. Could you have a look as well, Marcel? procedure TJvCustomAppXMLStorage.SetRootNodeName(const Value: string); begin if Value = '' then raise EPropertyError.CreateRes(@RsENodeCannotBeEmpty) else begin StringReplace(Value, ' ', '_', [rfReplaceAll]); Xml.Root.Name := Value; Root := Value; end; end; To make it work, you probably need to delete the old xml file first. edited on: 05-25-04 23:40 |
|
|
Olivier wrote the XMLStorage so he can better determine if it's correct. |
|
|
Updated in CVS but not quite sure this is the correct fix (might have unknown side-effects) |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2004-05-24 14:34 | bgn | New Issue | |
| 2004-05-24 14:34 | bgn | File Added: DockingLayout.zip | |
| 2004-05-25 02:23 |
|
Note Added: 0004342 | |
| 2004-05-25 02:23 |
|
Status | new => assigned |
| 2004-05-25 02:23 |
|
Assigned To | => user72 |
| 2004-05-25 05:54 | marcelb | Note Added: 0004343 | |
| 2004-05-25 06:29 |
|
Note Added: 0004344 | |
| 2004-05-25 06:39 | marcelb | Note Added: 0004345 | |
| 2004-05-25 10:45 |
|
Note Added: 0004347 | |
| 2004-05-25 11:01 | marcelb | Note Added: 0004349 | |
| 2004-05-25 11:41 | bgn | Note Added: 0004350 | |
| 2004-05-25 11:53 |
|
Note Added: 0004351 | |
| 2004-05-25 12:04 |
|
Note Added: 0004352 | |
| 2004-05-25 12:56 | marcelb | Note Added: 0004353 | |
| 2004-05-25 13:18 | bgn | Note Added: 0004354 | |
| 2004-05-25 14:18 |
|
Note Added: 0004356 | |
| 2004-05-25 22:49 | bgn | Note Added: 0004357 | |
| 2004-05-25 23:39 |
|
Note Added: 0004358 | |
| 2004-05-25 23:40 |
|
Note Edited: 0004358 | |
| 2004-05-26 01:36 | marcelb | Note Added: 0004359 | |
| 2004-05-28 01:07 |
|
Status | assigned => resolved |
| 2004-05-28 01:07 |
|
Resolution | open => fixed |
| 2004-05-28 01:07 |
|
Note Added: 0004402 |
