all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 32194@debbugs.gnu.org
Subject: bug#32194: [PATCH] Use Gnulib regex for lib-src
Date: Sat, 4 Aug 2018 16:34:33 -0700	[thread overview]
Message-ID: <2d8e4d2b-c6b1-4e39-dfb0-d10b75d9caf8@cs.ucla.edu> (raw)
In-Reply-To: <83a7q2t9zh.fsf@gnu.org>

Eli Zaretskii wrote:

> The idea is that the system-supplied regex library is also GNU regex,
> but one tuned better to the specific platform.  The configure script
> could check whether that library is up to speed, before using it.
> 
> Obviously, not a terribly important issue, but I've seen projects that
> do this.

I doubt whether any platform has a system-supplied regex library compatible with 
the glibc API and performing significantly better. The glibc code is hairy and 
was written by an expert and its behavior would be hard to replicate. In the 
unlikely event that there is such a platform and it requires unusual flags, that 
platform's builder can simply configure with the appropriate CFLAGS, LDLIBS, 
etc. We needn't worry about this unlikely event.

> I'd only ask that the
> renaming be done as a separate commit before the rest of the changes
> in those two files, so that all the changes in src/regex.[ch] will be
> after the rename, as that will make future forensics easier.

OK, will do.

>>> P.S. Is it true that this version will no longer support a build with
>>> WIDE_CHAR_SUPPORT undefined, i.e. those which have only the C locale?
>>
>> I don't see any issues with such a build. What sort of problem do you
>> have in mind?
> 
> AFAIU, you suggest removing the !WIDE_CHAR_SUPPORT code, but we
> previously supported platforms that don't have all the prerequisites
> for using that code.

If I understand you correctly, I doubt whether such platforms survive now. If 
they do we can add Gnulib-based substitutes for the missing prerequisites. I 
already did that in Bug#32194#5; you suggested in Bug#32194#8 point (3) that we 
not bother with it, though, and this seemed like good idea so that's what in the 
current proposal. If the idea doesn't work it will be easy to go back to the 
approach in Bug#32194#5 (as I recall it's a simple change to admin/merge-gnulib 
followed by turning the crank).

>> (malloc, realloc, free): Do not redefine.  Adjust all callers
>> to use xmalloc, xrealloc, xfree instead.
> 
> Is this wise?  xmalloc etc. are tuned to allocate Lisp data and
> aligned objects, something that isn't necessarily needed in regex
> library.  Why not use the normal memory allocation routines?

src/regex.c is already using Lisp allocation when compiled as part of Emacs, so 
this is just code simplification, not a real change. The Emacs regex code needs 
to use xmalloc to do the right thing with memory exhaustion and to do memory 
profiling.

> we should compare the performance of etags before and
> after the switch, just to be sure we aren't going to suffer a
> performance penalty.

I expect there to be a performance penalty but it's no big deal. On my old 
Fedora 28 x86-64 platform with 'make TAGS CFLAGS='-O2 -march=native"' in the 
Emacs src directory, user+system time grows from 0.55 to 0.59 seconds, about a 
10% slowdown. It grows about 10% more, to 0.64 seconds, if I use the 
system-installed regex library instead of the Gnulib-supplied one. I don't view 
an extra tenth of a second as a glitch big enough to be worth investigating.





  reply	other threads:[~2018-08-04 23:34 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-17 23:47 bug#32194: [PATCH] Use Gnulib regex for lib-src Paul Eggert
2018-07-18 16:34 ` Eli Zaretskii
2018-07-31 18:21   ` Paul Eggert
2018-08-01  0:29     ` Noam Postavsky
2018-08-01  0:59       ` Paul Eggert
2018-08-01  1:10         ` Noam Postavsky
2018-08-01  1:27           ` Paul Eggert
2018-08-02  0:58             ` Noam Postavsky
2018-08-02  1:49               ` Paul Eggert
2018-08-02  1:55                 ` Noam Postavsky
2018-08-02  2:42               ` Eli Zaretskii
2018-08-02  3:19                 ` Paul Eggert
2018-08-02 10:28                   ` Noam Postavsky
2018-08-02 14:05     ` Eli Zaretskii
2018-08-02 14:41       ` Paul Eggert
2018-08-02 14:55         ` Eli Zaretskii
2018-08-02 15:41           ` Paul Eggert
2018-08-04 15:41     ` Eli Zaretskii
2018-08-04 23:34       ` Paul Eggert [this message]
2018-08-05 17:58         ` Eli Zaretskii
2018-08-06  2:40           ` Paul Eggert
2018-08-06 10:59             ` Andy Moreton
2018-08-06 14:52               ` Eli Zaretskii
2018-08-06 15:40                 ` Andy Moreton
2018-08-06 14:54             ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2d8e4d2b-c6b1-4e39-dfb0-d10b75d9caf8@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=32194@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.