Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_element() need to be more efficient #128

Open
Elv13 opened this issue Jan 29, 2016 · 1 comment
Open

_element() need to be more efficient #128

Elv13 opened this issue Jan 29, 2016 · 1 comment

Comments

@Elv13
Copy link

Elv13 commented Jan 29, 2016

component.mt:_element is called hundreds of thousand of time per minute. It is where the Awesome spend ~50% of its time (LGI account for up to 97% of the Lua time). Speed has gotten better since 0.6, but it is still not enough. Here is some proposed changes. I would also have idea how to better reduce the numbers of calls (this only make them less expensive).

Here are some proposed changes and questions:

local match_cache = {}

-- Retrieves (element, category) pair from given componenttable and
-- instance for given symbol.
function component.mt:_element(instance, symbol)
   -- This generic version can work only with strings. Everything else
   -- will cause an error. It is better to let it happen as this is an
   -- hot code path and type() is slow

   -- Check keyword translation dictionary.  If the symbol can be
   -- found there, try to lookup translated symbol.
   --QUESTION: Is this really necessary? When does this happen
   --symbol = keyword_dictionary[symbol] or symbol

   -- Check whether symbol is directly accessible in the component.
   local element = rawget(self, symbol)
   if element then return element end

   -- Decompose symbol name, in case that it contains category prefix
   -- (e.g. '_field_name' when requesting explicitely field called
   -- name).
   local category, name = nil, nil

   -- string.gmatch is quite slow and the pool of symbol is quite small
   -- it seem preferable to cache them given this method is the most called
   -- by LGI
   local cached_symbol = match_cache[symbol]
   if not cached_symbol then
      category, name      = string.match(symbol, '^(_.-)_(.*)$')
      cached_symbol       = {category, name}
      match_cache[symbol] = cached_symbol
   else
      -- Cannot use unpack() as the table may be sparse and it is undefined
      category, name = cached_symbol[1], cached_symbol[2]
   end

   if category and name and category ~= '_access' then
      -- Check requested category.
      local cat = rawget(self, category)
      element = cat and cat[name]
   elseif string.sub(symbol, 1, 1) ~= '_' then

      -- Check the first category
      if category then
          local cat = rawget(self, category)
          element = cat and cat[symbol]

          -- Methods and enums are sypposed to be static, element can be nil
          -- but it is a NOP, so the "if" would be more expensive
          rawset(self, symbol, element)
      else
          local categories = self._categories or {}

          -- Check all available categories.
          for i = 1, #categories do
             category = categories[i]
             local cat = rawget(self, category)
             element = cat and cat[symbol]

             if element and (category == "_method" or category == "_enum" or category == "_union")then
                 cached_symbol[1] = category
                 break
             end

             if element then break end
          end
      end
   end
   if element then
      -- Make sure that table-based attributes have symbol name, so
      -- that potential errors contain the name of referenced
      -- attribute.
      if type(element) == 'table' and category == '_attribute' then
     element._name = element._name or symbol
      end
      return element, category
   end
end
  1. Avoid calling match as often, expressions/patterns are slow
  2. Cache the category and name
  3. rawset The methods and enums as (I don't think?) they will ever change
  4. Remove the type() check. Let it crash and fix the bug where it is rather than mitigate the consequence.
  5. Removed the check for "function" and "local". Why is it there?

Those changes change lua-callgrind benchmark from 16M to 10M total instruction cost for my Awesome WM config.
callgrind.zip

I attach the profile with the fixes. I can be used with most valgrind compatible tools such as KCacheGrind

I tried caching everything using rawset, but attributes and structs will assert LGI with "got userdata, expected record". Any idea to cache even more?

@Elv13
Copy link
Author

Elv13 commented Jan 29, 2017

Anniversary bump

edit: 3rd anniversary bump

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant