Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(678)

Issue 10933075: FYI: Remove PerModelSafeGroupState + move ConflictResolution (Closed)

Created:
8 years, 3 months ago by rlarocque
Modified:
8 years, 1 month ago
Reviewers:
CC:
chromium-reviews, Raghu Simha, ncarter (slow), haitaol1, akalin, tim (not reviewing)
Visibility:
Public.

Description

FYI: Remove PerModelSafeGroupState + move ConflictResolution I started this series of patches with the intent of moving conflict resolution to make it part of update application. I haven't figured out how to briefly explain why this makes sense. Feel free to ask me in person if you're curious. Along the way, it made sense to refactor the UpdateApplicator. I was struggling to understand it while I was modifying it, so I refactored the logic into something a bit more conventional. Part of the refactor eventually led to the removal of ConflictProgress. I also ended up merging the steps to Verify and Process updates. This allowed me to remove UpdateProgress as well. We might be able to find other savings as well; I suspec that reverifying entries is no longer necessary. However, it turns out this part part of the change wasn't required to achieve my goal, so it could be put back if necessary. Finally, I was able to merge the update application and conflict resolution. Results: - The HasMoreToSync() loop is no longer necessary. - Removes the last source of SYNC_CYCLE_CONTINUATION GUs, and should slightly reduce our commit QPS, too. - The number of syncer.cc states reduced by 3. - The number of ModelChangingSyncerCommands per cycle reduced by 3. - Net diff of -700 lines before making any significant changes to sync logic. - Net diff of -1200 lines once conflict resolution was moved. - These changes will make it much easier to implement commmit-only sync cycles. I have no intention of committing this all at once. I intend to break them up into separate patches, add a bit of polish, and commit them one at a time. Consider this a road map for the next month.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+493 lines, -1674 lines) Patch
M sync/engine/all_status.cc View 1 chunk +0 lines, -1 line 0 comments Download
M sync/engine/apply_updates_command.cc View 2 chunks +29 lines, -6 lines 0 comments Download
M sync/engine/apply_updates_command_unittest.cc View 12 chunks +26 lines, -116 lines 0 comments Download
M sync/engine/conflict_resolver.h View 3 chunks +1 line, -7 lines 0 comments Download
M sync/engine/conflict_resolver.cc View 3 chunks +8 lines, -8 lines 0 comments Download
M sync/engine/process_commit_response_command.cc View 3 chunks +1 line, -4 lines 0 comments Download
M sync/engine/process_updates_command.h View 1 chunk +5 lines, -0 lines 0 comments Download
M sync/engine/process_updates_command.cc View 2 chunks +179 lines, -26 lines 0 comments Download
M sync/engine/process_updates_command_unittest.cc View 3 chunks +55 lines, -8 lines 0 comments Download
D sync/engine/resolve_conflicts_command.h View 1 chunk +0 lines, -32 lines 0 comments Download
D sync/engine/resolve_conflicts_command.cc View 1 chunk +0 lines, -41 lines 0 comments Download
D sync/engine/resolve_conflicts_command_unittest.cc View 1 chunk +0 lines, -51 lines 0 comments Download
M sync/engine/sync_scheduler_impl.cc View 2 chunks +3 lines, -11 lines 0 comments Download
M sync/engine/sync_scheduler_unittest.cc View 1 chunk +0 lines, -30 lines 0 comments Download
M sync/engine/syncer.h View 1 chunk +0 lines, -2 lines 0 comments Download
M sync/engine/syncer.cc View 5 chunks +0 lines, -48 lines 0 comments Download
M sync/engine/syncer_proto_util_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M sync/engine/syncer_unittest.cc View 14 chunks +17 lines, -44 lines 0 comments Download
M sync/engine/syncer_util.h View 1 chunk +0 lines, -1 line 0 comments Download
M sync/engine/syncer_util.cc View 1 chunk +0 lines, -1 line 0 comments Download
M sync/engine/update_applicator.h View 2 chunks +13 lines, -53 lines 0 comments Download
M sync/engine/update_applicator.cc View 3 chunks +72 lines, -138 lines 0 comments Download
D sync/engine/verify_updates_command.h View 1 chunk +0 lines, -48 lines 0 comments Download
D sync/engine/verify_updates_command.cc View 1 chunk +0 lines, -210 lines 0 comments Download
D sync/engine/verify_updates_command_unittest.cc View 1 chunk +0 lines, -109 lines 0 comments Download
M sync/internal_api/debug_info_event_listener.h View 1 chunk +0 lines, -1 line 0 comments Download
M sync/internal_api/js_sync_manager_observer.cc View 1 chunk +0 lines, -1 line 0 comments Download
M sync/internal_api/public/sessions/model_neutral_state.h View 2 chunks +6 lines, -3 lines 0 comments Download
M sync/internal_api/public/sessions/model_neutral_state.cc View 1 chunk +4 lines, -1 line 0 comments Download
D sync/sessions/session_state.h View 1 chunk +0 lines, -133 lines 0 comments Download
D sync/sessions/session_state.cc View 1 chunk +0 lines, -142 lines 0 comments Download
M sync/sessions/session_state_unittest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M sync/sessions/status_controller.h View 6 chunks +28 lines, -55 lines 0 comments Download
M sync/sessions/status_controller.cc View 4 chunks +32 lines, -133 lines 0 comments Download
M sync/sessions/status_controller_unittest.cc View 3 chunks +6 lines, -65 lines 0 comments Download
M sync/sessions/sync_session.h View 6 chunks +2 lines, -19 lines 0 comments Download
M sync/sessions/sync_session.cc View 3 chunks +5 lines, -51 lines 0 comments Download
M sync/sessions/sync_session_unittest.cc View 5 chunks +0 lines, -53 lines 0 comments Download
M sync/sessions/test_util.h View 1 chunk +0 lines, -2 lines 0 comments Download
M sync/sessions/test_util.cc View 2 chunks +0 lines, -9 lines 0 comments Download
M sync/sync.gyp View 4 chunks +0 lines, -8 lines 0 comments Download

Powered by Google App Engine
This is Rietveld 408576698