[Lldb-commits] [PATCH] D68678: Speed up accelerator table lookups

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So now we are doing two lookups each time we are doing a lookup? One idea to improve upon this would be to do this search a bit later on after we lookup "iterator" and only do this extra context matching if we find at last a certain number of results? So the flow would go:

- do normal lookup on "iterator" and find N matches
- if N is large enough then do your search above?

See inline code for an example of a possible improvement



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:114
 
   // When searching for a scoped type (for exampe,
   // "std::vector<int>::const_iterator") searching for the innermost
----------------
s/exampe/example/


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:120-127
+  if (!has_qualified_name_hash && (context.GetSize() > 1) &&
+      (context[1].tag == DW_TAG_class_type ||
+       context[1].tag == DW_TAG_structure_type)) {
     DIEArray class_matches;
     m_apple_types_up->FindByName(context[1].name, class_matches);
     if (class_matches.empty())
       return;
----------------
This should probably be moved down into line 139 and 143 instead of checking "if (! has_qualified_name_hash..."? See inline comments below.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:138-143
   } else if (has_tag) {
     if (log)
       m_module.LogMessage(log, "FindByNameAndTag()");
     m_apple_types_up->FindByNameAndTag(type_name.GetStringRef(), tag, offsets);
   } else
     m_apple_types_up->FindByName(type_name.GetStringRef(), offsets);
----------------
We can probably move your code above to here:
```
  } else {
    if (context.GetSize() > 1) && (context[1].tag == DW_TAG_class_type ||
                                   context[1].tag == DW_TAG_structure_type)) {
      DIEArray class_matches;
      m_apple_types_up->FindByName(context[1].name, class_matches);
      if (class_matches.empty())
        return;
    }
    if (has_tag) {
      if (log)
        m_module.LogMessage(log, "FindByNameAndTag()");
      m_apple_types_up->FindByNameAndTag(type_name.GetStringRef(), tag, offsets);
    } else
      m_apple_types_up->FindByName(type_name.GetStringRef(), offsets);
  }
```

Or better yet:
```
 } else {
    if (has_tag) {
      if (log)
        m_module.LogMessage(log, "FindByNameAndTag()");
      m_apple_types_up->FindByNameAndTag(type_name.GetStringRef(), tag, offsets);
    } else
      m_apple_types_up->FindByName(type_name.GetStringRef(), offsets);
    // If we have too many matches, we might want to ensure that the context
    // that this type is contained in actually exists so we don't end up pulling 
    // in too much DWARF for common type like "iterator".
    if (offsets.size() > THRESHOLD) {
      if (context.GetSize() > 1) && (context[1].tag == DW_TAG_class_type ||
                                     context[1].tag == DW_TAG_structure_type)) {
        DIEArray class_matches;
        m_apple_types_up->FindByName(context[1].name, class_matches);
        if (class_matches.empty()) {
          offsets.clear();
          return;
        }
      }
    }
  }
```

We might also be able to remove some offsets from "offsets" by iterating through "offsets" and removing any values that come before an entry in "class_matches". For example if offsets contains:
```
offsets = [0x1100, 0x2100, 0x3100, 0x4100]
```
and
```
class_offsets = [0x4000]
```
could we remove "[0x1100, 0x2100, 0x3100]" from offsets? Maybe not with DW_AT_specification and DW_AT_abstract_origin?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68678/new/

https://reviews.llvm.org/D68678