NetBSD Problem Report #54213

From leot@netbsd.org  Fri May 17 20:19:32 2019
Return-Path: <leot@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id E145A7A186
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 17 May 2019 20:19:32 +0000 (UTC)
Message-Id: <20190517201932.9B2BD84DB5@mail.netbsd.org>
Date: Fri, 17 May 2019 20:19:32 +0000 (UTC)
From: leot@NetBSD.org
Reply-To: leot@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: Significant performance degradation of whatis(1) after src/usr.sbin/makemandb/whatis.c,-r1.6
X-Send-Pr-Version: 3.95

>Number:         54213
>Category:       misc
>Synopsis:       Significant performance degradation of whatis(1) after src/usr.sbin/makemandb/whatis.c,-r1.6
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    abhinav
>State:          closed
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Fri May 17 20:20:00 +0000 2019
>Closed-Date:    Sat May 18 10:39:33 +0000 2019
>Last-Modified:  Sat May 18 10:40:01 +0000 2019
>Originator:     Leonardo Taccari
>Release:        NetBSD 8.99.37
>Organization:
Università Politecnica delle Marche
>Environment:


System: NetBSD arm64 8.99.37 NetBSD 8.99.37 (GENERIC64) #0: Fri Apr 26 18:04:06 CEST 2019 leot@abacus:/tmp/obj/sys/arch/evbarm/compile/GENERIC64 evbarm
Architecture: aarch64
Machine: evbarm
>Description:
	After revision 1.6 of whatis.c:

	----------------------------
	revision 1.6
	date: 2017-04-23 18:56:49 +0200;  author: abhinav;  state: Exp;  lines: +10 -4;  commitid: tfkpTFPDR89xbGOz;
	Teach whatis(1) to handle MLINKS

	Similar to apropos(1), whatis did not utilise the mandb_links table till now.
	Therefore, if it was asked about one of the links to a man page, it would
	error out. This change teaches whatis(1) to look up both the FTS table
	as well as the links table, thus ensuring that it is able to answer queries
	about MLINKS as well.

	Comparision between outputs before this change and after this change:

	#Before change
	$ whatis realloc
	realloc: not found

	#after change
	$ ./whatis realloc
	realloc(3) - general memory allocation operations
	realloc(3) - general purpose memory allocation functions
	realloc(9) - general-purpose kernel memory allocator
	----------------------------

	whatis(1) performance degradated significantly.
>How-To-Repeat:
	(At the time of writing this the latest whatis.c revision is -r1.7.)

	% cd src/usr.sbin/makemandb
	% make whatis
	[...]
	% mv whatis whatis-1.7
	% cvs up -r1.5 -p whatis.c > whatis.c
	% make whatis
	% mv whatis whatis-1.5

	And then running whatis(1) 10 times against a query that produces no
	results (`foo'): 

	% time sh -c 'for i in $(seq 10); do { ./whatis-1.5 foo 2>/dev/null; } done'
	0.048u 0.305s 0:00.35 97.1%     45+1208k 0+0io 120pf+0w

	% time sh -c 'for i in $(seq 10); do { ./whatis-1.7 foo 2>/dev/null; } done'
	29.509u 2.826s 0:32.45 99.5%    0+16k 0+0io 120pf+0w

	(The times are the one of a Pinebook with a `/' on a uSD but the
	slowdowns can be recognized also on faster machines.) 
>Fix:
	No idea, sorry.

	If the SQL query could not be optimized it should be probably
	worth to add a way to disable MLINKS handling.

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: misc-bug-people->abhinav
Responsible-Changed-By: abhinav@NetBSD.org
Responsible-Changed-When: Sat, 18 May 2019 07:14:59 +0000
Responsible-Changed-Why:
I am looking into it


State-Changed-From-To: open->feedback
State-Changed-By: abhinav@NetBSD.org
State-Changed-When: Sat, 18 May 2019 07:58:39 +0000
State-Changed-Why:
Could you give the committed changes a try and see if it improves things?


From: "Abhinav Upadhyay" <abhinav@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54213 CVS commit: src/usr.sbin/makemandb
Date: Sat, 18 May 2019 07:56:44 +0000

 Module Name:	src
 Committed By:	abhinav
 Date:		Sat May 18 07:56:43 UTC 2019

 Modified Files:
 	src/usr.sbin/makemandb: apropos-utils.c apropos-utils.h makemandb.c
 	    whatis.c

 Log Message:
 PR misc/54213: Fix performance of whatis(1) when no matches are found

 In revision 1.6 of whatis.c the query was modified to return matches for names found
 in MLINKS of the man pages as well. However it was slow. The reason probably being that it
 required a join. But more importantly the where condition on an FTS virtual table column
 is very slow. To avoid the join and the expensive where condition on the virtual table,
 add the name_desc column to the mandb_links table as well. This improves the performance
 of whatis(1) to the original level at the expense of slight data duplication.

 Bump the schema to force database rebuild to take account for the new column addition


 To generate a diff of this commit:
 cvs rdiff -u -r1.43 -r1.44 src/usr.sbin/makemandb/apropos-utils.c
 cvs rdiff -u -r1.14 -r1.15 src/usr.sbin/makemandb/apropos-utils.h
 cvs rdiff -u -r1.59 -r1.60 src/usr.sbin/makemandb/makemandb.c
 cvs rdiff -u -r1.7 -r1.8 src/usr.sbin/makemandb/whatis.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Leonardo Taccari <leot@NetBSD.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: misc/54213 (Significant performance degradation of whatis(1) after src/usr.sbin/makemandb/whatis.c,-r1.6)
Date: Sat, 18 May 2019 12:11:47 +0200

 Hello Abhinav,

 abhinav@NetBSD.org writes:
 > Synopsis: Significant performance degradation of whatis(1) after src/usr=
 .sbin/makemandb/whatis.c,-r1.6
 >
 > State-Changed-From-To: open->feedback
 > State-Changed-By: abhinav@NetBSD.org
 > State-Changed-When: Sat, 18 May 2019 07:58:39 +0000
 > State-Changed-Why:
 > Could you give the committed changes a try and see if it improves things=
 ?

 Sure!

 Redoing all the tests with whatis-1.5, whatis-1.7 and whatis-1.8.
 First I've prepared a man.db prior today commit using a
 `/tmp/man.conf.old', redoing the tests for whatis -r1.5 and -r1.7:

  % time sh -c 'for i in $(seq 10); do { ./whatis-1.5 -C /tmp/man.conf.old =
 foo 2>/dev/null; } done'
  0.089u 0.234s 0:00.32 96.8%     50+1863k 0+0io 120pf+0w

  % time sh -c 'for i in $(seq 10); do { ./whatis-1.7 -C /tmp/man.conf.old =
 foo 2>/dev/null; } done'
  28.916u 2.403s 0:31.31 100.0%   0+15k 0+0io 120pf+0w

 ...then I've `cvs up' today changes, locally rebuilt makemandb and
 whatis and prepared a man.db using a `/tmp/man.conf', tests for
 whatis -r1.8:

  % time sh -c 'for i in $(seq 10); do { ./whatis-1.8 -C /tmp/man.conf foo =
 2>/dev/null; } done'
  0.057u 0.269s 0:00.32 96.8%     50+1551k 0+0io 120pf+0w

 So, yes, now the performance of -r1.8 are similar to the one of
 whatis.c -r1.5, thank you!

 I have also checked that the resulting man.db sizes are comparable
 and they are both 42MB.


 I think that whatis.c -r1.7 was accidentally reverted and case
 insensitive searchs no longer works, e.g.:

  % ./whatis-1.5 -C /tmp/man.conf.old ApRoPoS
  ApRoPoS: not found
  % ./whatis-1.7 -C /tmp/man.conf.old ApRoPoS
  apropos(1) - search the complete content of all man pages
  % ./whatis-1.8 -C /tmp/man.conf ApRoPoS
  ApRoPoS: not found

 Possible patch to reintroduce that:

  <https://www.NetBSD.org/~leot/src-patches/whatis-nocase.patch>

 If that's ok please let me know and I'll commit it (or feel free
 to commit it directly!).


 Apart that I think that also DBSCHEMA should be updated to document
 `name_desc' attribute of `mandb_links' table.

From: Abhinav Upadhyay <er.abhinav.upadhyay@gmail.com>
To: NetBSD GNATS <gnats-bugs@netbsd.org>
Cc: Abhinav Upadhyay <abhinav@netbsd.org>, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	Leonardo Taccari <leot@netbsd.org>
Subject: Re: misc/54213 (Significant performance degradation of whatis(1)
 after src/usr.sbin/makemandb/whatis.c,-r1.6)
Date: Sat, 18 May 2019 15:51:22 +0530

 On Sat, May 18, 2019 at 3:45 PM Leonardo Taccari <leot@netbsd.org> wrote:
 >
 > The following reply was made to PR misc/54213; it has been noted by GNATS.
 >
 > From: Leonardo Taccari <leot@NetBSD.org>
 > To: gnats-bugs@netbsd.org
 > Cc:
 > Subject: Re: misc/54213 (Significant performance degradation of whatis(1) after src/usr.sbin/makemandb/whatis.c,-r1.6)
 > Date: Sat, 18 May 2019 12:11:47 +0200
 >
 >  Hello Abhinav,
 >
 >  abhinav@NetBSD.org writes:
 >  > Synopsis: Significant performance degradation of whatis(1) after src/usr=
 >  .sbin/makemandb/whatis.c,-r1.6
 >  >
 >  > State-Changed-From-To: open->feedback
 >  > State-Changed-By: abhinav@NetBSD.org
 >  > State-Changed-When: Sat, 18 May 2019 07:58:39 +0000
 >  > State-Changed-Why:
 >  > Could you give the committed changes a try and see if it improves things=
 >  ?
 >
 >  Sure!
 >
 >  Redoing all the tests with whatis-1.5, whatis-1.7 and whatis-1.8.
 >  First I've prepared a man.db prior today commit using a
 >  `/tmp/man.conf.old', redoing the tests for whatis -r1.5 and -r1.7:
 >
 >   % time sh -c 'for i in $(seq 10); do { ./whatis-1.5 -C /tmp/man.conf.old =
 >  foo 2>/dev/null; } done'
 >   0.089u 0.234s 0:00.32 96.8%     50+1863k 0+0io 120pf+0w
 >
 >   % time sh -c 'for i in $(seq 10); do { ./whatis-1.7 -C /tmp/man.conf.old =
 >  foo 2>/dev/null; } done'
 >   28.916u 2.403s 0:31.31 100.0%   0+15k 0+0io 120pf+0w
 >
 >  ...then I've `cvs up' today changes, locally rebuilt makemandb and
 >  whatis and prepared a man.db using a `/tmp/man.conf', tests for
 >  whatis -r1.8:
 >
 >   % time sh -c 'for i in $(seq 10); do { ./whatis-1.8 -C /tmp/man.conf foo =
 >  2>/dev/null; } done'
 >   0.057u 0.269s 0:00.32 96.8%     50+1551k 0+0io 120pf+0w
 >
 >  So, yes, now the performance of -r1.8 are similar to the one of
 >  whatis.c -r1.5, thank you!
 >
 >  I have also checked that the resulting man.db sizes are comparable
 >  and they are both 42MB.
 >

 Great, thank you for verifying in detail :)
 >
 >  I think that whatis.c -r1.7 was accidentally reverted and case
 >  insensitive searchs no longer works, e.g.:
 >
 >   % ./whatis-1.5 -C /tmp/man.conf.old ApRoPoS
 >   ApRoPoS: not found
 >   % ./whatis-1.7 -C /tmp/man.conf.old ApRoPoS
 >   apropos(1) - search the complete content of all man pages
 >   % ./whatis-1.8 -C /tmp/man.conf ApRoPoS
 >   ApRoPoS: not found
 >
 >  Possible patch to reintroduce that:
 >
 >   <https://www.NetBSD.org/~leot/src-patches/whatis-nocase.patch>
 >
 >  If that's ok please let me know and I'll commit it (or feel free
 >  to commit it directly!).

 Yes, please commit it, thank you for noticing.
 >
 >  Apart that I think that also DBSCHEMA should be updated to document
 >  `name_desc' attribute of `mandb_links' table.

 Yes, good catch. If you get a chance do it or I will do when I get
 back to my machine.

 -
 Abhinav

From: Leonardo Taccari <leot@NetBSD.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: PR/54213 CVS commit: src/usr.sbin/makemandb
Date: Sat, 18 May 2019 12:24:28 +0200

 Abhinav Upadhyay writes:
 > [...]
 > Module Name:  src
 > Committed By: abhinav
 > Date:         Sat May 18 07:56:43 UTC 2019
 > 
 > Modified Files:
 >       src/usr.sbin/makemandb: apropos-utils.c apropos-utils.h makemandb.c
 >           whatis.c
 > 
 > Log Message:
 > PR misc/54213: Fix performance of whatis(1) when no matches are found
 > [...]

 Sorry, I forgot to document that in the PR - and only for completeness
 - performance degradation also happened for queries with matches
 (i.e. all queries, not just queries with no matches).

From: "Leonardo Taccari" <leot@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54213 CVS commit: src/usr.sbin/makemandb
Date: Sat, 18 May 2019 10:28:57 +0000

 Module Name:	src
 Committed By:	leot
 Date:		Sat May 18 10:28:57 UTC 2019

 Modified Files:
 	src/usr.sbin/makemandb: whatis.c

 Log Message:
 Reintroduce case insensitive comparison of name accidentally lost in last
 revision.

 Discussed with <abhinav> via PR misc/54213, thanks!


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/usr.sbin/makemandb/whatis.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: feedback->closed
State-Changed-By: leot@NetBSD.org
State-Changed-When: Sat, 18 May 2019 10:39:33 +0000
State-Changed-Why:
Problem fixed.

Thank you very much Abhinav!


From: "Leonardo Taccari" <leot@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54213 CVS commit: src/usr.sbin/makemandb
Date: Sat, 18 May 2019 10:38:04 +0000

 Module Name:	src
 Committed By:	leot
 Date:		Sat May 18 10:38:04 UTC 2019

 Modified Files:
 	src/usr.sbin/makemandb: DBSCHEMA

 Log Message:
 Document name_desc attribute of mandb_links.

 Discussed with <abhinav> via PR misc/54213, thanks!


 To generate a diff of this commit:
 cvs rdiff -u -r1.2 -r1.3 src/usr.sbin/makemandb/DBSCHEMA

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search