lgi-devs / lgi-devs/lgi

_element() need to be more efficient

Open
#128 1 comment 6 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Lua
Stars
491
Forks
81
PR merge metrics
No merged PRs in 30d

Description

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?

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the component.mt:_element entry point and inspect the attached callgrind profile with KCacheGrind. Compare the proposed symbol caching and rawset changes against the existing behavior, including the reported attribute and struct assertion failures. Done means lower benchmark instruction cost without those regressions.

Written by the indexing model from the issue text.

Assessment

Tech stack
lua
Domain
performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.