From nobody Thu Apr 20 06:54:46 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Q27gr3Psvz46Phd; Thu, 20 Apr 2023 06:54:48 +0000 (UTC) (envelope-from hselasky@freebsd.org) Received: from mail.turbocat.net (turbocat.net [IPv6:2a01:4f8:c17:6c4b::2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4Q27gq5RJRz45mD; Thu, 20 Apr 2023 06:54:47 +0000 (UTC) (envelope-from hselasky@freebsd.org) Authentication-Results: mx1.freebsd.org; none Received: from [10.36.2.154] (unknown [46.212.121.255]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id 73FC12601A5; Thu, 20 Apr 2023 08:54:46 +0200 (CEST) Message-ID: <7c37a671-ab84-2a03-cd1f-d0bb75f8d153@freebsd.org> Date: Thu, 20 Apr 2023 08:54:46 +0200 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: git: a7469c9c0a50 - main - libc: bsort_s() requires both __BSD_VISIBLE and __EXT1_VISIBLE Content-Language: en-US To: Brooks Davis Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202304192013.33JKDIrM070521@gitrepo.freebsd.org> From: Hans Petter Selasky In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 4Q27gq5RJRz45mD X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:24940, ipnet:2a01:4f8::/32, country:DE] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On 4/20/23 01:43, Brooks Davis wrote: > On Wed, Apr 19, 2023 at 11:35:56PM +0200, Hans Petter Selasky wrote: >> On 4/19/23 22:29, Brooks Davis wrote: >>> This is a formal request to revert all commits related to bsort. It >>> should not have been committed without much broader discussion and IMO >>> does not belong in the tree. It certainly should not be in the tree >>> under such a generic name. >>> >>> -- Brooks >> >> Hi Brooks, >> >> I don't have an issue reverting my bsort() patch series, but please >> clarify your statement first. Who are "we" this time, representing this >> formal request for revert? > > This is my request. Hi Brooks, Maybe bsort() can be an internal symbol to libc, I'm not sure if we support that. At the same time I want a solution forward about our qsort(). I would say that qsort() falling back to my bsort() would be a good solution, when qsort() already today sporadically exhibit O(N²) behaviour. How to do that clean? Regarding benchmarks, there is: https://github.com/hselasky/qsortbench It's basically a fork of a test-suite for sorting algorithms, having my bsort() added on top. There you also have the bad-cases for the FreeBSD's qsort(). My bsort() scores pretty OK on average. > > I see some review and the thread below, but adding > non-standard symbols that are likely to collide with other software[0] to > libc should be subject to a higher bar than a few people helping you > improve your patch of saying "that's neat". > > New things added to libc should be in a standard or aligned with one > (e.g., strlcpy, etc) and anything not from a standard should have > immediate uses where it improves things in the rest of the system. > Critically I don't see plans or prototype conversions and I don't see > benchmarks of real systems (which could easily be done with LD_PRELOAD. See answer above. > >> Regarding "broader discussion" - what do you mean? >> >> The initial discussion was started last September: >> >> https://lists.freebsd.org/archives/freebsd-arch/2022-September/000225.html > > More pushback here probably would be been good, sorry. A heads up > before the actual commit might have been a good idea. I personally > find your answer to the question, "why not improve qsort instead?" > unsatisfactory. It might be that importing your implementation makes > sense, but I don't think making is a public symbol we're stuck with > forever if we ship it in 14 is a good way to go. My plan was to get a broader audience for the bsort() algorithm, get it well tested to iron out any bugs in there, and then I see qsort() could fallback to bsort() as a remedy, which would be a great use-case. Regarding the libc symbol name, I could expand the "b" into "bitonic", but it will be like "int" vs "int32_t" to me. > >> And several people have been asked for review and comments. Please >> elaborate what "broader discussion" means? Do you mean like getting >> something into ANSI first? I don't get it. > > I'd like more "I'd use it for X" and less "that's neat". Do I have a commitment and plan from your side to work on this if I back the bsort() patch series out? I already see Jessica mentioning some memswap() patches, and I guess it is due to ongoing work for Cheri? I think we need something like bsort(). If it's not exactly like my version, something like that _is_ needed from what I can see. > > -- Brooks > > [0] Debian code search finds fewer collisions than I'd feared, but not > zero: https://codesearch.debian.net/search?q=%5B%5Ea-z%5Dbsort%5C%28&literal=0 Thanks for checking. I did some research already, and it doesn't look that bad. --HPS