From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753053AbbC3Pel (ORCPT <rfc822;w@1wt.eu>);
	Mon, 30 Mar 2015 11:34:41 -0400
Received: from mail-ie0-f181.google.com ([209.85.223.181]:33759 "EHLO
	mail-ie0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752263AbbC3Pei (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 30 Mar 2015 11:34:38 -0400
Message-ID: <55196D0C.8060105@gmail.com>
Date: Mon, 30 Mar 2015 09:34:36 -0600
From: David Ahern <dsahern@gmail.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.5.0
MIME-Version: 1.0
To: Jiri Olsa <jolsa@redhat.com>
CC: acme@kernel.org, linux-kernel@vger.kernel.org,
        Don Zickus <dzickus@redhat.com>, Joe Mario <jmario@redhat.com>
Subject: Re: [PATCH 1/2] perf tool: Refactor comm/tgid lookup
References: <1427668202-15422-1-git-send-email-dsahern@gmail.com> <20150330080132.GA8679@krava.brq.redhat.com>
In-Reply-To: <20150330080132.GA8679@krava.brq.redhat.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/30/15 2:01 AM, Jiri Olsa wrote:
> On Sun, Mar 29, 2015 at 04:30:01PM -0600, David Ahern wrote:
>
> SNIP
>
>> +	fd = open(filename, O_RDONLY);
>> +	if (fd < 0) {
>>   		pr_debug("couldn't open %s\n", filename);
>>   		return 0;
>>   	}
>>
>> -	while (!comm[0] || (tgid < 0)) {
>> -		if (fgets(bf, sizeof(bf), fp) == NULL) {
>> -			pr_warning("couldn't get COMM and pgid, malformed %s\n",
>> -				   filename);
>> -			break;
>> -		}
>> -
>> -		if (memcmp(bf, "Name:", 5) == 0) {
>> -			char *name = bf + 5;
>> -			while (*name && isspace(*name))
>> -				++name;
>> -			size = strlen(name) - 1;
>> -			if (size >= len)
>> -				size = len - 1;
>> -			memcpy(comm, name, size);
>> -			comm[size] = '\0';
>> -
>> -		} else if (memcmp(bf, "Tgid:", 5) == 0) {
>> -			char *tgids = bf + 5;
>> -			while (*tgids && isspace(*tgids))
>> -				++tgids;
>> -			tgid = atoi(tgids);
>> -		}
>> +	n = read(fd, bf, sizeof(bf) - 1);
>> +	close(fd);
>
> we have filename__read_str, which will read whole file into buffer,
> and you dont need to worry about the file size.. not that there's
> any worry wrt /proc/<pid>/status file, but who knows.. ;-)

hmmm.... I did not know about that function. But I don't think we want 
to use it for this case: This function is called a LOT (think 50k or 
100k threads) and we want to synthesize the threads as fast as possible. 
Using stack (like above) is much faster than dealing with malloc and 
free on each pass.

David