[Freeipa-devel] [PATCH 0112-7] Speeding up cli help

[David Kupka]

On 03/08/16 16:33, Jan Cholasta wrote:
> On 3.8.2016 16:23, David Kupka wrote:
>> On 21/07/16 10:12, Jan Cholasta wrote:
>>> Hi,
>>>
>>> On 20.7.2016 14:32, David Kupka wrote:
>>>> On 15/07/16 12:53, David Kupka wrote:
>>>>> Hello!
>>>>>
>>>>> After Honza introduced thin client that builds plugins and commands
>>>>> dynamically from schema client became much slower. This is only
>>>>> logical,
>>>>> instead of importing a module client now must fetch the schema from
>>>>> server, parse it and instantiate the commands using the data.
>>>>>
>>>>> First step to speed it up was addition of schema cache to client. That
>>>>> removed the RTT and download time of fetching schema every time.
>>>>>
>>>>> Now the most time consuming task became displaying help for lists of
>>>>> topics and command and displaying individual topics. This is simply
>>>>> because of the need to instantiate all the commands to find the
>>>>> relations between topics and commands.
>>>>>
>>>>> All the necessary bits for server commands and topics are already in
>>>>> the
>>>>> schema cache so we can skip this part and generate help from it,
>>>>> right?
>>>>> Not so fast!
>>>>>
>>>>> There are client plugins with commands and topics. So we can generate
>>>>> basic bits (list of all topics, list of all commands, list of commands
>>>>> for each topic) from schema and store it in cache. Then we need to go
>>>>> through all client plugins and get similar bits for client plugins.
>>>>> Then
>>>>> we can merge and print.
>>>>>
>>>>> Still the client response is not as fast as before and I this it even
>>>>> can't be. Also first time you display particular topic or list takes
>>>>> longer because it must be freshly generated and stored in cache for
>>>>> next
>>>>> use. And this is what the attached patches do.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/6048
>>>>
>>>> Reimplemented so there is no need to distinguish client plugins and
>>>> remote plugins.
>>>> The main idea of this approach is to avoid creating instances of the
>>>> commands just to get the information about topic, name and summary
>>>> needed for displaying help. Instead class properties are used to access
>>>> the information directly in schema.
>>>
>>> Patch 0112:
>>>
>>> I think this would better be done in Schema.read_namespace_member,
>>> because Schema is where all the state is.
>>>
>>> (BTW does _SchemaNameSpace.__getitem__ raise KeyError for non-existent
>>> keys? It looks like it doesn't.)
>>>
>>>
>>> Patch 0113:
>>>
>>> How about setting _schema_cached to False in Schema.__init__() rather
>>> that getattr()-ing it in _ensure_cached()?
>>>
>>>
>>> Patch 0116:
>>>
>>> ClientCommand.doc should be a class property as well, otherwise .summary
>>> won't work on it correctly.
>>>
>>> _SchemaCommand.doc should not be a property, as it's not needed for
>>> .summary to work on it correctly.
>>>
>>>
>>> Otherwise works fine for me.
>>>
>>> Honza
>>>
>>
>> Updated patches attached.
>
> Thanks, ACK.
>
> Pushed to master: 229e2a1ed9ea9877cb5e879fadd99f9040f77c96
>

I've made and reviewer overlooked some errors. Attached patches fix them.

-- 
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0119.0-schema-cache-Do-not-reset-ServerInfo-dirty-flag.patch
Type: text/x-patch
Size: 1131 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160804/71520513/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0120.0-schema-cache-Write-format-information-in-cache.patch
Type: text/x-patch
Size: 1029 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160804/71520513/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0121.0-Access-data-for-help-separately.patch
Type: text/x-patch
Size: 4102 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160804/71520513/attachment-0002.bin>