Use DeeIndex instead of strcmp in PlaceEntryHome

Bug #724886 reported by Neil J. Patel on 2011-02-25
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Unity
Fix Released
Medium
Neil J. Patel
unity (Ubuntu)
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 (didrocks) on 2011-02-25
Changed in unity (Ubuntu):
status: New → Triaged
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
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  Edit
Everyone can see this information.

Other bug subscribers