User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008061712 Fedora/3.0-1.fc9 Firefox/3.0
Build Identifier: 0.1.30
Second machine I installed weave on goes to 100% CPU and only syncs bookmarks and history (I have asked it to sync everything).
Reproducible: Always
Steps to Reproduce:
1. I installed Weave on one machine and set it to save bookmarks, history, cookies, passwords, form-data.
2. I then installed weave on a second machine and logged on and successfully synced bookmarks+history.
3. Then I checkmarked cookies, passwords and formdata on that machine and tried to sync again.
Actual Results:
I got 100% CPU usage for about 40 minutes on second machine. Only bookmarks and history seem to be being synced on that machine. They are the only weave/snapshot files on that machine. Killing firefox and restarting returned after a while to 100% cpu usage and cookies, formdata and passwords are still not being synced or saved in the snapshot files.
Firefox+Weave on the machine I first installed weave seem to be fine.
Firefox+Weave on the machine I first installed weave seem to be fine.
Expected Results:
Second machine should import new passwords that are there on the first machine and should not use 100%cpu.

Getting the same issue with syncing passwords and/or form data on a second computer. All other engines appear to be syncing without exhibiting this behavior.
This happened with 0.1.30 as well as 0.1.32.
It happened with 0.1.30 with a Vista machine (FF3 release version) as the first machine and the second machine XP (FF3 release version) was the one getting stuck at 100% CPU 'forever' as weave attempted to sync passwords and/or form data (all other engines worked fine)
It happened with 0.1.32 with an XP machine (FF3 release version) as the first machine and the second machine Vista (FF3 release version) was the one getting stuck at 100% CPU 'forever' as weave attempted to sync passwords and/or form data (all other engines worked fine)
I can dig deeper and provide more details and log output if necessary.

For now, we will disable form history and password sync. Both seem to cause large reconciliation runs which are impossible to complete in any reasonable amount of time.
Putting this in the queue for 0.2, we may find some solution anyway (which would allow us to re-enable those engines).

Created attachment 327193[details][diff][review]
Improves perf. of FormEngine by using an in-memory lookup table
This patch maintains an in-memory representation of the wrapped JSON for quicker lookups. The lookup table is passed from the FormStore to the FormSyncCore by the Engine superclass, right after it calls wrap but before calling reconcile (which in turn calls _itemExists).

Comment on attachment 327193[details][diff][review]
Improves perf. of FormEngine by using an in-memory lookup table
>+ // wrappedJSON is set to the result of wrap by the engine
>+ if (this.wrappedJSON[GUID])
>+ return true;
>+ else
>+ return false;
I think 'if (GUID in this.wrappedJSON)' is more correct. Otherwise you are depending on the object/value it points to being true.

Created attachment 327199[details][diff][review]
Make _itemExists a method of the Store, and pass a reference to syncCore so it can be called
We move _itemExists to Store, and pass a reference to it when a syncCore is constructed to make sure it can be called when reconcile is invoked.
_itemExists can, as a result, directly lookup the table which is now a local property of the Store.

Created attachment 327202[details][diff][review]
Default implementation of _itemExists, and ensuring wrap sets the _lookup property of the Store
Store now has a default implementation of _itemExists, which assumes that wrap has previously set the _lookup property with GUIDs as keys. _itemExists now only needs to be overridden if necessary (as in the case of History and Tab engines).

Created attachment 327203[details][diff][review]
Default implementation of _itemExists, and ensuring wrap sets the _lookup property of the Store
Throwing instead of logging, and commented wrap in stores.js as per previous comment.