dee

Optimize DBus API for Dee

Bug #622454 reported by Mikkel Kamstrup Erlandsen
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Unity
Fix Released
Medium
Mikkel Kamstrup Erlandsen
dee
Fix Released
Medium
Mikkel Kamstrup Erlandsen
unity (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

TRANSACTION BASED PROTOCOL
Currently the DeeModel DBus protocol has signals RowsAdded, RowsChanged, and RowsRemoved. This means that we can not support transactions in the wire protocol as well as many ops require two dbus messages. For example in the case of updating Unity search results we first fire off RowsRemoved to invalidate any non-matching rows, and then a RowsAdded with the new hits.

We need a protocol where this can be done in one message. I am thinking one signal like:

  Commit(s swarm_name, aav row_data, au positions, ay change_code, (tt) seqnum_before_after)

Here row_data, positions, and seqnums are as in the current protocol, but the new arg 'change_code' is an enumeration with the values:

 0: Add
 1: Change
 2: Remove

All the arrays in the Commit signal *must* have the same length, and like we also have currently in the protocol the positions vector is applied sequentially with each offset taking into account any previous row changes in the transaction affecting rows before it.

This would also mandate two new GInterfaces; DeeTransactionalModel and DeeTransactionModel that this builds upon. The transactional model could have one new method:

  DeeTransactionModel dee_transactional_model_begin_transaction (DeeTransactionalModel *model);

And DeeTransactionModel is a proxy model (extending DeeProxyModel to help the impl) that buffers changes until someone calls:

  void dee_transaction_model_commit (DeeTransactionModel *transaction);

In order to cancel a transaction one would simply unref the transaction.

FIXME: This approach would incur a lot of memory reallocation (all ops are essentially done twice). Perhaps the are clever workarounds...

MORE OPTIMIZED MESSAGE ROUTING
In the DeeModel protocol we use for Maverick the DBus signals RowsAdded/Removed/Changed does not pass the swarm name they apply to. This means that each swarm member must keep track of all swarm members and subscribe to their signals. Which means installing a new match rule for each swarm member and doing a lot of book keeping in the code.

If we instead mimicked what we already do in the DeePeer protocol we could pass the swarm name as the first argument of the signals. This way swarm members need only ever install one match rule ala:

  interface='com.canonical.Dee.Model',type='signal',arg0='$swarmname'

This match rule would pick up all signals relevant to that swarm and nothing else.

PEER -2-PEER DBUS
In GDBus it's fairly easy to set up a peer-2-peer connection. This should save us the extra socket roundtrip we have when the DBus daemon is involved.

Tags: backlog

Related branches

Changed in dee:
assignee: nobody → Mikkel Kamstrup Erlandsen (kamstrup)
importance: Undecided → Wishlist
status: New → Triaged
summary: - Transaction based DBus API for Dee
+ Optimize DBus API for Dee
description: updated
Changed in dee:
milestone: none → 0.5.0
Changed in dee:
importance: Wishlist → Medium
David Barth (dbarth)
Changed in unity:
status: New → Triaged
importance: Undecided → Medium
assignee: nobody → Mikkel Kamstrup Erlandsen (kamstrup)
tags: added: backlog
Neil J. Patel (njpatel)
Changed in unity:
milestone: none → 3.4
Changed in dee:
status: Triaged → In Progress
Changed in unity:
status: Triaged → In Progress
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Changing the Commit signal from:

  Commit(av row_data, au positions, au change_code, at seqnums)

to:

  Commit(av row_data, au positions, ay change_code, (tt) seqnum_before_after)

So 'change_code' becomes just one byte, and we don't transfer all the intermediate seqnums because they *must* be in sequential order (n, n+1, n+2, n+3,...) anyway so it's redundant info. These to changes cuts significantly don't on the message sizes.

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Woops. So row_data should have been 'aav' in all of the above

description: updated
Changed in unity:
status: In Progress → Fix Released
Changed in dee:
status: In Progress → Fix Released
Neil J. Patel (njpatel)
Changed in unity:
milestone: 3.4 → 3.2.12
Changed in unity (Ubuntu):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.