Re: [PATCH] lisp/org.el: Obsolete `org-cached-entry-get' in favor of `org-entry-get'

[Morgan Smith]

Ihor Radchenko <yantar92@posteo.net> writes:

> Morgan Smith <morgan.j.smith@outlook.com> writes:
>>> Also, with the old approach, if you observe slowdowns, you likely have
>>> some property being calculated slowly (like BLOCKED in my case). Do you
>>> happen to know which property is it for your setup?
>>
>> According to my profiler, I think it's using 30% of the CPU time during
>> my custom org-clock-sum just to get ITEM.  I suppose it's because it
>> thinks it has to grab and cache everything when all I'm after is ITEM.
>
> Not sure here. Getting ITEM is just a single regexp match.
> May you share the profiler report? (M-x profiler-report-write-profile)

I'm experiencing some bugs with `profiler-find-profile'.  Might be
related to all the type changes that happened recently in Emacs master
(I like to run on the bleeding edge so I can experience as much pain and
frustration as possible).

Anyways, it's not just a regexp match since `org-cached-entry-get' tells
`org-entry-properties' to get everything.  We can see here almost all of
the slowdown for me occurs because of `org-element-context' which is
called in the `find-ts' lambda that search's for TIMESTAMP and
TIMESTAMP_IA.

8780  33%                    - org-cached-entry-get
8780  33%                     - org-entry-properties
8696  32%                      - #<byte-code-function 775>
8236  31%                       + org-element-context


The comment written just above `org-element-properties-map' says this:

#+BEGIN_SRC elisp
;; There is purposely no function like `org-element-properties' that
;; returns a list of properties.  Such function would tempt the users
;; to (1) run it, creating a whole new list; (2) filter over that list
;; - the process requiring a lot of extra consing, adding a load onto
;; Emacs GC, memory used, and slowing things up as creating new lists
;; is not free for CPU.
#+END_SRC

This implies that the function `org-entry-properties' is just a bad
idea.  Although giving it an argument for WHICH does make it better.

It seems like it's only used in about 4 places in our codebase so if we
wanted to go further, we could potentially see more performance
enhancements if we obsolete this function as well.

I haven't really looked into it much though so I apologize if my
analyses is wrong.  Which it definitely could be.