Unity

Use DeeIndex instead of strcmp in PlaceEntryHome

Bug #724886 reported by Neil J. Patel
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Unity
Fix Released
Medium
Neil J. Patel
Unity 3.6.6
unity (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

I was just skimming through the dash-fixes diff and one thing caught my
eye. As far as I can understand you merge all the results from the
global place models into _results_model right?

If that is true then this function looks pretty expensive to me:

void
PlaceEntryHome::OnResultRemoved (DeeModel *model, DeeModelIter *it,
PlaceEntryHome *self)
{
  DeeModelIter *iter, *end;
  const char *uri;

  uri = dee_model_get_string (model, it, RESULT_URI);

  iter = dee_model_get_first_iter (self->_results_model);
  end = dee_model_get_last_iter (self->_results_model);
  while (iter != end)
  {
    if (g_strcmp0 (dee_model_get_string (self->_results_model, iter,
RESULT_URI), uri) == 0)
    {
      dee_model_remove (self->_results_model, iter);
      break;
    }

    iter = dee_model_next (self->_results_model, iter);
  }
}

You can avoid the iterations and g_strcmp()s completely by using
DeeIndex. So create an index on the RESULT_URI column with something
ala:

In the PlaceEntryHome constructor create the index (storing the index in
a member, not a local though):

  DeeAnalyzer *analyzer = dee_analyzer_new_for_key_column (RESULT_URI);
  DeeIndex *uri_index = dee_hash_index_new (_results_model, analyzer);
  g_free (analyzer);

And in PlaceEntryHome::OnResultRemoved():

  uri = dee_model_get_string (model, it, RESULT_URI);
  DeeResultSet *rows = dee_index_lookup (results_index,
                                         uri,
                                         DEE_TERM_MATCH_EXACT);
  while (dee_result_set_has_next (rows))
    {
      DeeModelIter* row = dee_result_set_next (rows);
      dee_model_remove (_results_model, row);
    }

This should make removals O(1) assuming that there is a 1:1
correspondence between URIs and rows in the model. Which there roughly
is in practice.

Cheers,
Mikkel

Didier Roche-Tolomelli (didrocks)
Changed in unity (Ubuntu):
status: New → Triaged
Revision history for this message
Neil J. Patel (njpatel) wrote :

I don't use this method, however I took this slowness into account when redesigning the internal Places API in Unity and the lookups are much, much faster now in global search.

Changed in unity:
milestone: 3.8 → 3.6.6
status: Triaged → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (4.8 KiB)

This bug was fixed in the package unity - 3.6.6-0ubuntu1

---------------
unity (3.6.6-0ubuntu1) natty; urgency=low

  * New upstream release:
    - drag and drop from dash to launcher issues (LP: #727675)
    - unity-panel-service crashed in g_type_class_meta_marshal (LP: #727788)
    - Dash: first-use text entry does not work until Alt-F2 version is used
      (LP: #735342)
    - compiz crashed with SIGSEGV in g_type_check_instance_is_a()
      (LP: #734721)
    - drag from dash to launcher (LP: #662616)
    - Dash - Create single widget for Dashboard (LP: #683729)
    - [FFe] Recently installed applications should be easy to run
      (LP: #670403)
    - Pressing Alt key does not underline mnemonics (LP: #689179)
    - indicator-appmenu breaks Alt accelerator keys (LP: #663030)
    - can't pin KTouch to the launcher (LP: #693755)
    - Unity paints multiple times with multi-screen (LP: #727116)
    - unitymtgrabhandles crashes when no key is set (LP: #727144)
    - SIGSEGV in PlaceEntryHome::SetSearch (LP: #731927)
    - [launcher] New Default favorites (LP: #714707)
    - Dash: Alt-F2 Pressing enter on the dash can start the first entry of the
      second group from the history (LP: #734738)
    - Dash: keyboard arrow navigation disappears off bottom (LP: #735347)
    - Increase the size of the top left Launcher reveal area from 1px to a 3px
      by 3px square (LP: #736034)
    - [dash] scrollbar's clickable zone should extend to the right border of
      the dash border (LP: #651398)
    - Launcher - Replace Dash lens Launcher icons with updated versions
      (LP: #676613)
    - NuxUtilAccessible requires to implement support to key event listeners
      (LP: #702672)
    - launcher icons dnd doesn't behave correctly when the dash is in use
      (LP: #708885)
    - Keyboard-navigation: highlight stays after losing focus (LP: #713632)
    - Implement AtkComponent for ATK objects in the panel service
      (LP: #715297)
    - Super shortcuts for application place and worskspace swither conflicts
      with compiz keys (LP: #723273)
    - Use DeeIndex instead of strcmp in PlaceEntryHome (LP: #724886)
    - [dash] text cursor not vertically centred in the entry (LP: #724927)
    - Moving cursor in dash text entry field causes cursor artifacts
      (LP: #725493)
    - Missing "children_changed" event emission from the atk support
      (LP: #727137)
    - Typing should immediately search; focusing the search field is fiddly
      (LP: #727295)
    - "Find Internet Apps", "Browse the Web", and "Check Mail" are scattered
      in default Dash (LP: #729009)
    - Press-holding on a icon in the Launcher should de-couple the icon and
      enable the user to reorder the icon vertically without leaving the
      Launcher. (LP: #727922)
    - "Shortcuts" heading in Dash seems pointless (LP: #729000)
    - intellihide is incompatible with totem fullscreen / Still some false
      positive on ws switch (LP: #730679)
    - Launcher - provide visual design for launcher keyboard navigation
      (LP: #702490)
    - Dash - Update the 'Shortcuts' dash home icon (LP: #689763)
    - The Unity panel service Does not connect to the
      INDICATOR_OBJECT...

Read more...

Changed in unity (Ubuntu):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

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