Bug 1422127

Summary: Performance regression of cmdline processing
Product: [Community] LVM and device-mapper Reporter: Zdenek Kabelac <zkabelac>
Component: lvm2Assignee: Zdenek Kabelac <zkabelac>
lvm2 sub component: Command-line tools QA Contact: cluster-qe <cluster-qe>
Status: POST --- Docs Contact:
Severity: unspecified    
Priority: unspecified CC: agk, heinzm, jbrassow, msnitzer, prajnoha, teigland, thornber, zkabelac
Version: 2.02.169Flags: rule-engine: lvm-technical-solution?
rule-engine: lvm-test-coverage?
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Zdenek Kabelac 2017-02-14 14:17:20 UTC
Description of problem:

Current new cmdline advanced logic brought in noticeable performance regression - a whole lvm2 test suite runtime basically doubled.

Also the size of lvm2 binary and library has grown-up by ~1/2 MB.


Version-Release number of selected component (if applicable):
2.02.169

How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

Comment 1 David Teigland 2017-02-14 17:45:06 UTC
Could you try this with the parsing optimization I committed here:
https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=c0f2a59993e373781c6dd5568cd559547cfe49a1

The size is probably an effect of embedding the command defs text into the run time, which is the tradeoff that was requested vs the original design of build time processing.

Comment 2 Zdenek Kabelac 2017-02-15 10:25:36 UTC
So  test suite timing gets more optimistic - we are now at ~19% increase of timing before new parser logic - so it's good progress.

As for the size - the header file itself seems to be around 40K so it must be a lot of new code and debug info around likely behind 500K size increase (without equivalent code removal in other parts of lvm2) yet.

Comment 3 Zdenek Kabelac 2017-02-17 14:41:37 UTC
So in my valgrind it now appears with today's upstream HEAD - we are nearly on pair with previous speed - massive number of 'strcmp()' calls is now gone.

The only remaining issue is quite noticeable bigger binary size which is possibly behind couple % degradation of BB speed.

We likely could optimize string layout for better sharing of 'common' string parts, also we may possibly use more fixed string[] buffers for smaller/shorter string instead of having reloc entries and lots of internal pointers.

Comment 4 Zdenek Kabelac 2021-03-03 12:00:44 UTC
Pushed some further improvements on initialization times with the use of binary search - we are likely close to limits of what can be done with 'easily' with current code in this patch:

https://listman.redhat.com/archives/lvm-devel/2021-March/msg00020.html

Further reduction likely would be more efficient by precompiling 'command-line.in' directly into includable structure - so we avoid the whole unnecessary parsing in runtime in this case.

But that's for possible another RFE if there is a demand.

That said - there are ATM other areas of code worth for optimizing.