Hello! For IVI, I am working on cloud syncing. When syncing against a WebDAV server, SyncEvolution runs a two-way sync with binfile client on the WebDAV side and a server with admin data handled by SyncEvolution on the other side.
One goal for IVI is to minimize or better, avoid disk writes, because flash storage must last as long as possible. The most common case is that nothing changed on either side. In this case, libsynthesis unnecessarily updates nonce (even if not used; I've already patched that (1)), sync anchors (again, I have a patch for this: skip writing of admin data after detecting the special case (2)) and the change log in the binfile client. This last write happens in TBinfileImplDS::changeLogPreflight(). The changes are minor, just a few bytes change. I suspect that these are the time stamps and modcount embedded in the log. Would it be possible to check in changeLogPreflight() how significant the changes are? If there were no item changes, what would be the effect of not updating the header? There are two cases where that can happen with the attached patch: - the processes crashes - SyncEvolution skips the session shutdown, see (2). (1) For local sync, <requestedauth>none</requestedauth> and <requiredauth>none</requiredauth> are used. Patch attached. Okay? (2) This happens in SyncEvolution: the SaveAdminData implementation detects that nothing happened during the sync, then returns an error to libsynthesis and tells the SyncEvolution event loop to stop the sync without sending the final reply message to the client. The server side then finishes the sync successfully while the client side simply aborts without doing its own session shutdown. This also eliminates SaveAdminData on the binfile client side. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter.
>From 4683fceb72cef0e7301b735a23b1f40323da52ba Mon Sep 17 00:00:00 2001 From: Patrick Ohly <[email protected]> Date: Fri, 29 Aug 2014 10:35:47 +0200 Subject: [PATCH 1/2] syncsession: avoid unnecessary nonce update When the configure auth type needs no nonce, don't generate one and return NULL immediately, like newChallenge() would do. That avoids unnecessary disk writes for storing the new nonce, which is important for local syncs in IVI. It also avoids debug output about a challenge that would not get sent. --- src/sysync/syncsession.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/sysync/syncsession.cpp b/src/sysync/syncsession.cpp index 54c119c..f7c7a11 100644 --- a/src/sysync/syncsession.cpp +++ b/src/sysync/syncsession.cpp @@ -4061,15 +4061,22 @@ bool TSyncSession::processSyncOpItem( // generate challenge for session SmlChalPtr_t TSyncSession::newSessionChallenge(void) { + sysync::TAuthTypes auth = requestedAuthType(); + + // Avoid misleading debug output (there is no challenge) + // and more importantly, creating a new nonce that is not + // going to be used. That causes unnecessary disk writes. + if (auth == sysync::auth_none) return NULL; + string nonce; getNextNonce(fRemoteURI.c_str(),nonce); PDEBUGPRINTFX(DBG_PROTO,( "Challenge for next auth: AuthType=%s, Nonce='%s', binary %sallowed", - authTypeSyncMLNames[requestedAuthType()], + authTypeSyncMLNames[auth], nonce.c_str(), getEncoding()==SML_WBXML ? "" : "NOT " )); - return newChallenge(requestedAuthType(),nonce,getEncoding()==SML_WBXML); + return newChallenge(auth,nonce,getEncoding()==SML_WBXML); } // TSyncSession::newSessionChallenge -- 2.1.0.rc1
>From 115cef8a0ec1823b58808bbb9820f7cb82c2a313 Mon Sep 17 00:00:00 2001 From: Patrick Ohly <[email protected]> Date: Fri, 29 Aug 2014 10:39:17 +0200 Subject: [PATCH 2/2] binfileclient: avoid disk writes in changeLogPreflight() During a normal sync where nothing changed, only the header gets updated. This change is not critical and thus does not have to be flushed to disk unless also some entries get added or updated. The advantage is that when SyncEvolution detects a sync where nothing changed on either side and skips the client's session shutdown, the .bfi is left unchanged, which reduces flash wearout. To detect item changes, a brute-force byte comparison is used. This requires less changes to the code and is less error-prone than adding "modified=true" to all places where "existingentries" gets modified. --- src/sysync/binfileimplds.cpp | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/sysync/binfileimplds.cpp b/src/sysync/binfileimplds.cpp index 314ccc7..947fa0c 100755 --- a/src/sysync/binfileimplds.cpp +++ b/src/sysync/binfileimplds.cpp @@ -741,9 +741,9 @@ localstatus TBinfileImplDS::changeLogPreflight(bool &aValidChangelog) lsd.c_str() )); #endif - // - save header - err = fChangeLog.flushHeader(); - if (err!=BFE_OK) goto done; + // - force saving of header only if some entry changes + bool modified=false; + string origentries; // - we don't need the changelog to be updated when all we do is refresh from server if (isRefreshOnly()) goto done; // done ok // - load entire existing changelog into memory @@ -751,10 +751,12 @@ localstatus TBinfileImplDS::changeLogPreflight(bool &aValidChangelog) PDEBUGPRINTFX(DBG_ADMIN+DBG_DBAPI,("changeLogPreflight: at start, changelog has %ld entries",(long)numexistinglogentries)); if (numexistinglogentries>0) { // - allocate array for all existing entries (use sysync_malloc because e.g. on PalmOS this uses special funcs to allow > 64k) - existingentries = (TChangeLogEntry *)sysync_malloc(sizeof(TChangeLogEntry)*numexistinglogentries); + size_t entriessize = sizeof(TChangeLogEntry)*numexistinglogentries; + existingentries = (TChangeLogEntry *)sysync_malloc(entriessize); if (!existingentries) { err=BFE_MEMORY; goto done; } // not enough memory // - read all entries fChangeLog.readRecord(0,existingentries,numexistinglogentries); + origentries.assign((char *)existingentries, entriessize); // Mark all not-yet-deleted in the log as delete candidate // (those that still exist will be get the candidate mark removed below) for (logindex=0;logindex<numexistinglogentries;logindex++) { @@ -952,6 +954,7 @@ localstatus TBinfileImplDS::changeLogPreflight(bool &aValidChangelog) // create if entry is new if (!chgentryexists) { // this is a new, additional entry (and not a resurrected deleted one) + modified=true; fChangeLog.newRecord(currentEntryP); PDEBUGPRINTFX(DBG_ADMIN+DBG_DBAPI,("- item was newly added (no entry existed in changelog before)")); } @@ -1033,7 +1036,18 @@ localstatus TBinfileImplDS::changeLogPreflight(bool &aValidChangelog) } #endif // - write back all existing entries - fChangeLog.updateRecord(0,existingentries,numexistinglogentries); + if (existingentries && + memcmp(existingentries, origentries.c_str(), origentries.size())) { + fChangeLog.updateRecord(0,existingentries,numexistinglogentries); + modified=true; + } + + // Also write updated header if (and only if) something changed. + if (modified) { + err = fChangeLog.flushHeader(); + if (err!=BFE_OK) goto done; + } + // - now we can confirm we have a valid changelog aValidChangelog=true; DEBUGPRINTFX(DBG_ADMIN+DBG_DBAPI+DBG_EXOTIC,("changeLogPreflight: seen=%ld, fNumberOfLocalChanges=%ld",(long)seen,(long)fNumberOfLocalChanges)); -- 2.1.0.rc1
_______________________________________________ os-libsynthesis mailing list [email protected] http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
