220 41397 <7340ec43-7d0a-44e8-b38e-49220b0bb5b1@isocpp.org> article Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Nicol Bolas Newsgroups: gmane.comp.lang.c++.isocpp.proposals Subject: Re: Standard Proposal: Addition of a filter to recursive_directory_iterator Date: Thu, 24 Jan 2019 12:33:43 -0800 (PST) Approved: news@gmane.org Message-ID: <7340ec43-7d0a-44e8-b38e-49220b0bb5b1@isocpp.org> References: Reply-To: std-proposals@isocpp.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_498_706470879.1548362023312" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="270170"; mail-complaints-to="usenet@blaine.gmane.org" Cc: noel.tchidjo@gmail.com To: ISO C++ Standard - Future Proposals Original-X-From: std-proposals+bncBCEKFTV6ZUMBBKGCVDRAKGQE7DYIPIY@isocpp.org Thu Jan 24 21:33:47 2019 Return-path: Envelope-to: gclcip-std-proposals@m.gmane.org Original-Received: from mail-yb1-f197.google.com ([209.85.219.197]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.89) (envelope-from ) id 1gmlh5-00186E-7u for gclcip-std-proposals@m.gmane.org; Thu, 24 Jan 2019 21:33:47 +0100 Original-Received: by mail-yb1-f197.google.com with SMTP id n201sf1638606ybg.9 for ; Thu, 24 Jan 2019 12:33:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=isocpp-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:message-id:in-reply-to:references:subject :mime-version:x-original-sender:reply-to:precedence:mailing-list :list-id:list-post:list-help:list-archive:list-subscribe :list-unsubscribe; bh=4nTfZVIVobta/pU+fmwUPAC3C00OWmjF8TZSlYepK5Y=; b=oNHRbQfQZ4Ocem33ELltcxBMWNOhCvPMawEtrJby4ET67SKxg5jaOmrqB2/Raym5FJ W+lgEe8GsV/u+gdWcelPd1KrfKRIRjneEDDdMjab1iTdFqeaCXB7Y04QoMJhLQUqz3Vv fa+ZEzPaPP/HOVljNWJ1OJzXQfQ2rsvTL6+hv7LAbrtfeNPuGJ8X2okNSEdVH+GOoIY9 qBj7biNBuS9AS2jLdkdnb5T7+B+bjkh0UcOqo1AWWgX+Q87+PBlg6vmBFQmKY7c1+4dG 1+0T2TATaoB1wZyNMtwGTrBaskeAQcjLlSRQbuE6s9xRorJfYA/Fo4Ds6CPoRqSyCzaq 4tPQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:message-id:in-reply-to:references:subject :mime-version:x-original-sender:reply-to:precedence:mailing-list :list-id:list-post:list-help:list-archive:list-subscribe :list-unsubscribe; bh=4nTfZVIVobta/pU+fmwUPAC3C00OWmjF8TZSlYepK5Y=; b=oPaPCOiQm7+DN7m2IF9CsFdn7A6a7CLKO5ICW9v7S80X57fOSErmrQX80GqscEInZY Xj3VwLRNsIIP3v7ZKLbVWzfvSxV/ljjRgLQJ7D7R3CgBPwZC2XwUrUvXyO1QQl5YMCVp lvusuxh3448Bfv1YJRFfJt2AEJfzfFrmxL1UbkEnvNrU5otFHk29savVVkZF+6mlkz8U OZLd4duGuCYA5o05dMW+aPbhn1T6C7O8hPZdI+mggmK7ZxHkSkoBghLqrOhVlDt45mbA MhMWCpW5uJjxnQJ4l329AopDdsW23aCgncEHyCXHD9410O7YpmADUxJcIGa2qRypUTTy NA/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:message-id:in-reply-to :references:subject:mime-version:x-original-sender:reply-to :precedence:mailing-list:list-id:x-spam-checked-in-group:list-post :list-help:list-archive:list-subscribe:list-unsubscribe; bh=4nTfZVIVobta/pU+fmwUPAC3C00OWmjF8TZSlYepK5Y=; b=lc4NDwAibmOZwpEccG7wDmOjLv5bApgJ6a858Lf303RLB5GwaqWHTMGj2Ojf0Z+aeF CQjBcEuHGrGyiemGigvSkOXVJ1G+7gpySUhyVXJvth/MENwf5h+tjowsANCRxwKmiIQc eXPOGLvnSr4DXtCsPz0SQ2g7fbeaNjdjvgYhgah/8lOWfuE6iNxW9CBk1ahUiP+97llR 4dnSSvHfyRw1oEildzZdvK36rF66Al8UKeQ0sqfRmMG1FTt/8U3BBj4VdE+jAwxwx8z3 5GxbyaIIITXYzX0bO43jXcH8GdohHr/aEwzBc+dUeZ9akZWJ5Ym3gwmtvv+tV8qIF7xz X3kQ== X-Gm-Message-State: AJcUuke1Hclb0flqD1MKCfxBNq2Db7LLzYwgg/0qC5RPpgA6nOedAzO9 2EYu+GDW6z32yEAvJw9OEJc+aQ== X-Google-Smtp-Source: ALg8bN5hM5ymaF66ugOZoaxlXv/TucIvAMZgdqhrs333GrdK4hZer0/uvpNsBUagdIZwbBesYziAcA== X-Received: by 2002:a81:6c4e:: with SMTP id h75mr4207349ywc.12.1548362025402; Thu, 24 Jan 2019 12:33:45 -0800 (PST) X-BeenThere: std-proposals@isocpp.org Original-Received: by 2002:a25:b8d:: with SMTP id 135ls1994905ybl.9.gmail; Thu, 24 Jan 2019 12:33:44 -0800 (PST) X-Received: by 2002:a25:234a:: with SMTP id j71mr27072ybj.2.1548362023935; Thu, 24 Jan 2019 12:33:43 -0800 (PST) In-Reply-To: X-Original-Sender: jmckesson@gmail.com Precedence: list Mailing-list: list std-proposals@isocpp.org; contact std-proposals+owners@isocpp.org List-ID: X-Google-Group-Id: 399137483710 List-Post: , List-Help: , List-Archive: List-Subscribe: , List-Unsubscribe: , Xref: news.gmane.org gmane.comp.lang.c++.isocpp.proposals:41397 Archived-At: ------=_Part_498_706470879.1548362023312 Content-Type: multipart/alternative; boundary="----=_Part_499_108574529.1548362023313" ------=_Part_499_108574529.1548362023313 Content-Type: text/plain; charset="UTF-8" The specification states that the filtering function you provide must take a `path`. Which means that the internal system must generate a `path` object to pass to it. This necessitates that the string taken from the OS filesystem internals must be copied into the `path` object to be passed to the filter. And it should be noted that building a `path` from a string is not just a copy; the `path` implementation may need to adjust the data. And that's fine. What is *not* fine is that your profiling tests do not recognize this reality. Your "lambda" version takes a `const char*`, which is a internal string that has not yet been copied into a `path` and may never be copied into one if the filter doesn't pass. So there is no copy into a `path`, no generalizing of the string data, nada. Even the Win32 version of your code misses stuff. It does a UTF-16 to UTF-8 conversion, so that the filtering based on `const char*` can work. So there is a copy, but you copy into a fixed-sized stack object rather than potentially allocating memory. And you copy doesn't do any path fixup; it's in the Windows native format, which your code then filters. This is unlike what you might get from the `path`-based version of the filter. Yet your "current" version that you profile against does the copy into a path for each entry to be filtered against (just like the specification states will happen in the filtered version). In fact, it also does a needless *second copy* when it extracts the path from the iterator. But the other two have no such copy into a `path`. Basically, your profiling data is profiling one case, while you're asking for the specification to be changed to allow a substantially different case. I think that at least some of the performance gains from the internally filtered versions you're seeing is due to the fact that you only generate a `path` for paths post-filtering. You need to profile exactly what your technical specification asks for. Oh, and your example is broken, since `path::extension` returns a new path object containing the extension. So calling `c_str()` on that temporary creates a dangling reference. -- You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Future Proposals" group. To unsubscribe from this group and stop receiving emails from it, send an email to std-proposals+unsubscribe@isocpp.org. To post to this group, send email to std-proposals@isocpp.org. To view this discussion on the web visit https://groups.google.com/a/isocpp.org/d/msgid/std-proposals/7340ec43-7d0a-44e8-b38e-49220b0bb5b1%40isocpp.org. ------=_Part_499_108574529.1548362023313 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
The specification states that the filterin= g function you provide must take a `path`. Which means that the internal sy= stem must generate a `path` object to pass to it. This necessitates that th= e string taken from the OS filesystem internals must be copied into the `pa= th` object to be passed to the filter. And it should be noted that building= a `path` from a string is not just a copy; the `path` implementation may n= eed to adjust the data.

And that's fine.

What is not fine is that your profiling test= s do not recognize this reality. Your "lambda" version takes a `c= onst char*`, which is a internal string that has not yet been copied into a= `path` and may never be copied into one if the filter doesn't pass. So= there is no copy into a `path`, no generalizing of the string data, nada.<= /div>

Even the Win32 version of your code misses stuff. = It does a UTF-16 to UTF-8 conversion, so that the filtering based on `const= char*` can work. So there is a copy, but you copy into a fixed-sized stack= object rather than potentially allocating memory. And you copy doesn't= do any path fixup; it's in the Windows native format, which your code = then filters. This is unlike what you might get from the `path`-based versi= on of the filter.

Yet your "current"= version that you profile against does the copy into a path for each entry = to be filtered against (just like the specification states will happen in t= he filtered version). In fact, it also does a needless second copy w= hen it extracts the path from the iterator. But the other two have no such = copy into a `path`.

Basically, your profiling data= is profiling one case, while you're asking for the specification to be= changed to allow a substantially different case. I think that at least som= e of the performance gains from the internally filtered versions you're= seeing is due to the fact that you only generate a `path` for paths post-f= iltering.

You need to profile exactly what your te= chnical specification asks for.

Oh, and your examp= le is broken, since `path::extension` returns a new path object containing = the extension. So calling `c_str()` on that temporary creates a dangling re= ference.

--
You received this message because you are subscribed to the Google Groups &= quot;ISO C++ Standard - Future Proposals" group.
To unsubscribe from this group and stop receiving emails from it, send an e= mail to std-proposa= ls+unsubscribe@isocpp.org.
To post to this group, send email to std-proposals@isocpp.org.
To view this discussion on the web visit https://groups.google.= com/a/isocpp.org/d/msgid/std-proposals/7340ec43-7d0a-44e8-b38e-49220b0bb5b1= %40isocpp.org.
------=_Part_499_108574529.1548362023313-- ------=_Part_498_706470879.1548362023312-- .