Lua 5.4.2 New assorted suggestions

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
29 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Lua 5.4.2 New assorted suggestions

Ranier Vilela-2
Hi,

Here new assorted suggestions for Lua 5.4.2.

1. Standardization strncmp:  (src/lauxlib.c)
We know the size of the "method" constant in the function luaL_argerror.

diff --git a/lauxlib.c b/lauxlib.c
index 73504389..a4be6298 100644
--- a/lauxlib.c
+++ b/lauxlib.c
@@ -177,7 +177,7 @@ LUALIB_API int luaL_argerror (lua_State *L, int arg, const char *extramsg) {
   if (!lua_getstack(L, 0, &ar))  /* no stack frame? */
     return luaL_error(L, "bad argument #%d (%s)", arg, extramsg);
   lua_getinfo(L, "n", &ar);
-  if (strcmp(ar.namewhat, "method") == 0) {
+  if (strncmp(ar.namewhat, "method", 6) == 0) {
     arg--;  /* do not count 'self' */
     if (arg == 0)  /* error is in the self argument itself? */
       return luaL_error(L, "calling '%s' on bad self (%s)",

2. Standardization strncmp:  (src/lauxlib.c)
Is possible to get the size of the variable name, with the standard Lua functions.

@@ -359,11 +359,16 @@ LUALIB_API void *luaL_checkudata (lua_State *L, int ud, const char *tname) {
 
 LUALIB_API int luaL_checkoption (lua_State *L, int arg, const char *def,
                                  const char *const lst[]) {
-  const char *name = (def) ? luaL_optstring(L, arg, def) :
-                             luaL_checkstring(L, arg);
+  const char *name;
+  size_t l;
   int i;
+  if (def) {
+  name = luaL_optlstring(L, arg, def, &l);
+  } else {
+  name = luaL_checklstring(L, arg, &l);
+  }
   for (i=0; lst[i]; i++)
-    if (strcmp(lst[i], name) == 0)
+    if (strncmp(lst[i], name, l) == 0)
       return i;
   return luaL_argerror(L, arg,
                        lua_pushfstring(L, "invalid option '%s'", name));

3. Standardization strncmp:  (src/lauxlib.c)
We know the sizes of the "on" and "off" constants in the function checkcontrol .

@@ -1024,9 +1029,9 @@ static int checkcontrol (lua_State *L, const char *message, int tocont) {
   if (tocont || *(message++) != '@')  /* not a control message? */
     return 0;
   else {
-    if (strcmp(message, "off") == 0)
+    if (strncmp(message, "off", 3) == 0)
       lua_setwarnf(L, warnfoff, L);  /* turn warnings off */
-    else if (strcmp(message, "on") == 0)
+    else if (strncmp(message, "on", 2) == 0)
       lua_setwarnf(L, warnfon, L);   /* turn warnings on */
     return 1;  /* it was a control message */
   }

4. Avoid strlen at lua_pushstring, with a constant "k".

diff --git a/ldblib.c b/ldblib.c
index 5a326ade..4330e00c 100644
--- a/ldblib.c
+++ b/ldblib.c
@@ -377,7 +377,7 @@ static int db_sethook (lua_State *L) {
   }
   if (!luaL_getsubtable(L, LUA_REGISTRYINDEX, HOOKKEY)) {
     /* table just created; initialize it */
-    lua_pushstring(L, "k");
+    lua_pushlstring(L, "k", 1);
     lua_setfield(L, -2, "__mode");  /** hooktable.__mode = "k" */
     lua_pushvalue(L, -1);
     lua_setmetatable(L, -2);  /* metatable(hooktable) = hooktable */

5. fgets returns char * pointer which needs to be compared to NULL, not 0.
We know the size of the "cont\n" constant.

@@ -418,10 +418,10 @@ static int db_gethook (lua_State *L) {
 
 static int db_debug (lua_State *L) {
   for (;;) {
-    char buffer[250];
+    char buffer[256];
     lua_writestringerror("%s", "lua_debug> ");
-    if (fgets(buffer, sizeof(buffer), stdin) == 0 ||
-        strcmp(buffer, "cont\n") == 0)
+    if (fgets(buffer, sizeof(buffer), stdin) == NULL ||
+        strncmp(buffer, "cont\n", 5) == 0)
       return 0;
     if (luaL_loadbuffer(L, buffer, strlen(buffer), "=(debug command)") ||
         lua_pcall(L, 0, 0, 0))

6. Standardization strncmp.
We know the size of the "*t" constant.

diff --git a/loslib.c b/loslib.c
index e65e188b..a664a607 100644
--- a/loslib.c
+++ b/loslib.c
@@ -316,7 +316,7 @@ static int os_date (lua_State *L) {
   if (stm == NULL)  /* invalid date? */
     return luaL_error(L,
                  "date result cannot be represented in this installation");
-  if (strcmp(s, "*t") == 0) {
+  if (strncmp(s, "*t", 2) == 0) {
     lua_createtable(L, 0, 9);  /* 9 = number of fields */
     setallfields(L, stm);
   }

7. Standardization strncmp.
We know the sizes of the "const" and "close" constants.

diff --git a/lparser.c b/lparser.c
index 77813a74..e14fff03 100644
--- a/lparser.c
+++ b/lparser.c
@@ -1691,9 +1691,9 @@ static int getlocalattribute (LexState *ls) {
   if (testnext(ls, '<')) {
     const char *attr = getstr(str_checkname(ls));
     checknext(ls, '>');
-    if (strcmp(attr, "const") == 0)
+    if (strncmp(attr, "const", 5) == 0)
       return RDKCONST;  /* read-only variable */
-    else if (strcmp(attr, "close") == 0)
+    else if (strncmp(attr, "close", 5) == 0)
       return RDKTOCLOSE;  /* to-be-closed variable */
     else
       luaK_semerror(ls,

8. Avoid loop test against '\0' at strcpy.
We know the size of the var lenmod.

diff --git a/lstrlib.c b/lstrlib.c
index 940a14ca..3a09f88c 100644
--- a/lstrlib.c
+++ b/lstrlib.c
@@ -1215,7 +1215,8 @@ static void addlenmod (char *form, const char *lenmod) {
   size_t l = strlen(form);
   size_t lm = strlen(lenmod);
   char spec = form[l - 1];
-  strcpy(form + l - 1, lenmod);
+  memcpy(form + l - 1, lenmod, lm);
   form[l + lm - 1] = spec;
   form[l + lm] = '\0';
 }

9. Standardization strncmp.
We know the size of the EOFMARK constant.
Strchr is it's more efficient than strcmp.

diff --git a/lua.c b/lua.c
index b5b884b6..857e98cb 100644
--- a/lua.c
+++ b/lua.c
@@ -220,7 +220,7 @@ static int pushargs (lua_State *L) {
 static int handle_script (lua_State *L, char **argv) {
   int status;
   const char *fname = argv[0];
-  if (strcmp(fname, "-") == 0 && strcmp(argv[-1], "--") != 0)
+  if ((strchr(fname, '-') != NULL) && strncmp(argv[-1], "--", 2) != 0)
     fname = NULL;  /* stdin */
   status = luaL_loadfile(L, fname);
   if (status == LUA_OK) {
@@ -444,7 +444,7 @@ static int incomplete (lua_State *L, int status) {
   if (status == LUA_ERRSYNTAX) {
     size_t lmsg;
     const char *msg = lua_tolstring(L, -1, &lmsg);
-    if (lmsg >= marklen && strcmp(msg + lmsg - marklen, EOFMARK) == 0) {
+    if (lmsg >= marklen && strncmp(msg + lmsg - marklen, EOFMARK, sizeof(EOFMARK) - 1) == 0) {
       lua_pop(L, 1);
       return 1;
     }

regards,
Ranier Vilela

all.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Andrew Gierth
>>>>> "Ranier" == Ranier Vilela <[hidden email]> writes:

 Ranier> -  if (strcmp(ar.namewhat, "method") == 0) {
 Ranier> +  if (strncmp(ar.namewhat, "method", 6) == 0) {

That's obviously completely wrong, since it would then match any string
_starting with_ "method".

All your other examples of strncmp are wrong for the same reason.

 Ranier> 4. Avoid strlen at lua_pushstring, with a constant "k".

 Ranier> -    lua_pushstring(L, "k");
 Ranier> +    lua_pushlstring(L, "k", 1);

This really isn't likely to be worth it.

 Ranier> 5. fgets returns char * pointer which needs to be compared to
 Ranier> NULL, not 0.

0 is a valid way to say "the null pointer".

 Ranier> Strchr is it's more efficient than strcmp.

It also does something completely different, which is not applicable
here.

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Sean Conner
It was thus said that the Great Andrew Gierth once stated:
> >>>>> "Ranier" == Ranier Vilela <[hidden email]> writes:
>
>  Ranier> 5. fgets returns char * pointer which needs to be compared to
>  Ranier> NULL, not 0.
>
> 0 is a valid way to say "the null pointer".

  I would agree with Ranier with replacing the 0 with NULL to clarify the
intent, and to make it stand out, but I can understand why the Lua team
choose to use 0 in this instance (because saying "XXXXXXXX to C++
programmers", as I am wont to do, may be a bit extreme).

  -spc
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Ranier Vilela-2
In reply to this post by Andrew Gierth
Em dom., 15 de nov. de 2020 às 22:17, Andrew Gierth <[hidden email]> escreveu:
>>>>> "Ranier" == Ranier Vilela <[hidden email]> writes:

 Ranier> -  if (strcmp(ar.namewhat, "method") == 0) {
 Ranier> +  if (strncmp(ar.namewhat, "method", 6) == 0) {

That's obviously completely wrong, since it would then match any string
_starting with_ "method".
I still want to hear the Lua Team.
But if that is really the case, it is simple to resolve and continue to avoid calling strlen over and over again.
+if (strncmp(ar.namewhat, "method\0", 7) == 0) {


All your other examples of strncmp are wrong for the same reason.
Same above.


 Ranier> 4. Avoid strlen at lua_pushstring, with a constant "k".

 Ranier> -    lua_pushstring(L, "k");
 Ranier> +    lua_pushlstring(L, "k", 1);

This really isn't likely to be worth it.
Standardization.


 Ranier> 5. fgets returns char * pointer which needs to be compared to
 Ranier> NULL, not 0.

0 is a valid way to say "the null pointer".
Really.
And that talks about NULL.
In addition, in Lua fgets it is used against NULL.
See at:
File lua.c:
         fgets (b, LUA_MAXINPUT, stdin)! = NULL) / * get line * /


 Ranier> Strchr is it's more efficient than strcmp.

It also does something completely different, which is not applicable
here.
Basically, it does a search for '-', which strchr is much more efficient.
In addition, strchr is used elsewhere in the code to do the same.

Ranier Vilela
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Andre Leiradella-2
Hi Ranier,

I suggest you thoroughly test your changes and benchmark them before
suggesting changes to the Lua source code, some won't give you any
performance gains (and are likely even slower), and some others are
plain wrong.

Cheers,

Andre


On 16/11/20 09:37, Ranier Vilela wrote:

> Em dom., 15 de nov. de 2020 às 22:17, Andrew Gierth
> <[hidden email] <mailto:[hidden email]>>
> escreveu:
>
>     >>>>> "Ranier" == Ranier Vilela <[hidden email]
>     <mailto:[hidden email]>> writes:
>
>      Ranier> -  if (strcmp(ar.namewhat, "method") == 0) {
>      Ranier> +  if (strncmp(ar.namewhat, "method", 6) == 0) {
>
>     That's obviously completely wrong, since it would then match any
>     string
>     _starting with_ "method".
>
> I still want to hear the Lua Team.
> But if that is really the case, it is simple to resolve and continue
> to avoid calling strlen over and over again.
> +if (strncmp(ar.namewhat, "method\0", 7) == 0) {
>
>
>     All your other examples of strncmp are wrong for the same reason.
>
> Same above.
>
>
>      Ranier> 4. Avoid strlen at lua_pushstring, with a constant "k".
>
>      Ranier> -    lua_pushstring(L, "k");
>      Ranier> +    lua_pushlstring(L, "k", 1);
>
>     This really isn't likely to be worth it.
>
> Standardization.
>
>
>      Ranier> 5. fgets returns char * pointer which needs to be compared to
>      Ranier> NULL, not 0.
>
>     0 is a valid way to say "the null pointer".
>
> Really.
> And that talks about NULL.
> In addition, in Lua fgets it is used against NULL.
> See at:
> File lua.c:
>          fgets (b, LUA_MAXINPUT, stdin)! = NULL) / * get line * /
>
>
>      Ranier> Strchr is it's more efficient than strcmp.
>
>     It also does something completely different, which is not applicable
>     here.
>
> Basically, it does a search for '-', which strchr is much more efficient.
> In addition, strchr is used elsewhere in the code to do the same.
>
> Ranier Vilela
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Sean Conner
In reply to this post by Ranier Vilela-2
It was thus said that the Great Ranier Vilela once stated:
> Em dom., 15 de nov. de 2020 às 22:17, Andrew Gierth <
>
> >  Ranier> Strchr is it's more efficient than strcmp.
> >
> > It also does something completely different, which is not applicable
> > here.
>
> Basically, it does a search for '-', which strchr is much more efficient.
> In addition, strchr is used elsewhere in the code to do the same.

  The bit in question:

--- a/lua.c
+++ b/lua.c
@@ -220,7 +220,7 @@ static int pushargs (lua_State *L) {
 static int handle_script (lua_State *L, char **argv) {
   int status;
   const char *fname = argv[0];
-  if (strcmp(fname, "-") == 0 && strcmp(argv[-1], "--") != 0)
+  if ((strchr(fname, '-') != NULL) && strncmp(argv[-1], "--", 2) != 0)
     fname = NULL;  /* stdin */
   status = luaL_loadfile(L, fname);
   if (status == LUA_OK) {

strcmp() *compares* two strings for equivalence---it is comparing fname to
see if it is "-", *not* that it contains a '-', which is what strchr() does.
That's two different things.

  And if string comparisons are of such performance important, you might as
well skip strncmp() entirely (which still has to check for a NUL byte
anyway) and use a straight memcmp():

        if (strcmp(fname, "-") == 0 && memcmp(argv[-1], "--",2) != 0)

  My question to you is then---have you actually made these changes, run the
Lua regression test to see if it passes, *and* profile the code to see if it
indeed makes any difference to the speed?  If you are pushing for these
changes, then I would think it would be up to you to perform such work, if
only to make it easier for the Lua team to accept the patches.

  -spc
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Sean Conner
It was thus said that the Great Sean Conner once stated:
>
>   And if string comparisons are of such performance important, you might as
> well skip strncmp() entirely (which still has to check for a NUL byte
> anyway) and use a straight memcmp():
>
> if (strcmp(fname, "-") == 0 && memcmp(argv[-1], "--",2) != 0)

  Okay, three things wrong here---first off, I'm failed to include the
trailing NUL byte.  Second, I missed a strcmp()

        if (memcmp(fname,"-",2) == 0 && memcmp(argv[-1],"--",3) != 0)

  Third thing, this is *still* wrong in general.  Let's take an earlier
example:

        if (strcmp(ar.namewhat, "method") == 0) {

  If this is changed to memcmp(), as I suggested:

        if (memcmp(ar.namewhat,"method",7) == 0) {

If ar.namewhat is shorter than 7 characters (including the NUL byte) then
you run the risk of reading unknown memory, which invokes undefined behavior
in C.  I mean, it'll *probably* be okay in practice, but valgrind would
complain bitterly about reading uninitialized memory when testing.  You
still need to check for a NUL byte on both strings.

  So I rescind my recommendation to use memcmp().  I mean, I got *three*
things wrong in the span of *one* line of C code.  There's a reason the code
is written the way it is.

  -spc
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Julien Cugnière
In reply to this post by Ranier Vilela-2
Le lun. 16 nov. 2020 à 10:38, Ranier Vilela <[hidden email]> a écrit :
> I still want to hear the Lua Team.
> But if that is really the case, it is simple to resolve and continue to avoid calling strlen over and over again.
> +if (strncmp(ar.namewhat, "method\0", 7) == 0) {

I'm not the Lua Team, but : you seem to assume strcmp calls strlen,
and that strncmp is an optimization of strcmp. That is not the case :
* strcmp compares chars one by one, and stops when a difference is
encountered, or \0 is encountered. strlen is *not* called.
* strncmp compares chars one by one, and stops when a difference is
encountered, or n chars have been compared.

So using strncmp won't reduce the number of chars compared. It will
however make the code less readable, and more brittle (you have to
make sure the magic number 7 is correct, and stays in sync with the
string literal in case of changes).

--
Julien Cugnière
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Ranier Vilela-2
Em seg., 16 de nov. de 2020 às 07:34, Julien Cugnière <[hidden email]> escreveu:
Le lun. 16 nov. 2020 à 10:38, Ranier Vilela <[hidden email]> a écrit :
> I still want to hear the Lua Team.
> But if that is really the case, it is simple to resolve and continue to avoid calling strlen over and over again.
> +if (strncmp(ar.namewhat, "method\0", 7) == 0) {

* strncmp compares chars one by one, and stops when a difference is
encountered, or n chars have been compared.
Exactly. That's what strncmp was created for ("n chars have been compared").
I usually use sizeof, but it seems to me that Lua Team likes numerals more.

I usually use it like this:
+  if (strncmp(ar.namewhat, "method", sizeof("method")) == 0) { /* to compare exactly "method" */
or
+  if (strncmp(ar.namewhat, "method", sizeof("method") - 1) == 0) { /* to compare any "method" */


So using strncmp won't reduce the number of chars compared. It will
however make the code less readable, and more brittle (you have to
make sure the magic number 7 is correct, and stays in sync with the
string literal in case of changes).
sizeof makes it more readable. Nginx, uses a lot.

regards,
Ranier Vilela
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Ranier Vilela-2
In reply to this post by Andre Leiradella-2
Em seg., 16 de nov. de 2020 às 07:04, Andre Leiradella <[hidden email]> escreveu:
Hi Ranier,

I suggest you thoroughly test your changes and benchmark them before
suggesting changes to the Lua source code, some won't give you any
performance gains (and are likely even slower).
I do not agree.
If you disagree, you could run benchmarks and prove me wrong.

, and some others are plain wrong.
I still don't agree. I would like to hear about the Lua Team.

Ranier Vilela
v
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

v
On Mon, 2020-11-16 at 08:15 -0300, Ranier Vilela wrote:
> > Hi Ranier,
> >
> > I suggest you thoroughly test your changes and benchmark them
> > before
> > suggesting changes to the Lua source code, some won't give you any
> > performance gains (and are likely even slower).
> >
> I do not agree.
> If you disagree, you could run benchmarks and prove me wrong.
You're the one suggesting changes here, so it seems reasonable for you
to be one running benchmarks, preferably before posting suggestions.
Why would people include changes that appear to be unjustified?

> > , and some others are plain wrong.
> >
> I still don't agree. I would like to hear about the Lua Team.

Can you point out why you find the disputed ones correct? There is no
need to ask specific people if code objectivly won't do what it should,
which can be deduced by reading documentation to used functions.

If you specifically want to hear from Lua team, I'd expect changes to
come well-tested and benchmarked, since Lua team's time is valuable and
it is obviously smaller than active part of this mailing list.

--
v <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Francisco Olarte
In reply to this post by Ranier Vilela-2
Ranier:

On Mon, Nov 16, 2020 at 12:15 PM Ranier Vilela <[hidden email]> wrote:
> Em seg., 16 de nov. de 2020 às 07:04, Andre Leiradella <[hidden email]> escreveu:
>> I suggest you thoroughly test your changes and benchmark them before
>> suggesting changes to the Lua source code, some won't give you any
>> performance gains (and are likely even slower).
> I do not agree.
> If you disagree, you could run benchmarks and prove me wrong.

As an observer, I would normally expect the burden of the proof to the
one suggesting changes.

Anyway, what I was going to send. Are you aware that many of these
micro optimizations are cargo cult from a bygone age of simpler
compilers?
I mean, I just use stock debian, which does not have the more cutting
edge optmizations, and even then if I compile:
>>>>
#include <string.h>

int main(int ac, char **av) {
  /* I'm not going to run it but need to avoid getting all optimized
to a constant. */
  int a = strcmp(av[1],"method");
  int b = strncmp(av[2],"method", sizeof("method"));
  return a+b;
}
<<<<

using a plain gcc -S -O3 i get:

>>>>
        .file   "kk.c"
        .text
        .section        .rodata.str1.1,"aMS",@progbits,1
.LC0:
        .string "method"
        .section        .text.startup,"ax",@progbits
        .p2align 4,,15
        .globl  main
        .type   main, @function
main:
.LFB0:
        .cfi_startproc
        movq    %rsi, %r8
        leaq    .LC0(%rip), %rdx
        movq    8(%rsi), %rsi
        movl    $7, %ecx
        movq    %rdx, %rdi
        repz cmpsb
        movq    16(%r8), %rsi
        movq    %rdx, %rdi
        movl    $7, %ecx
        seta    %al
        sbbb    $0, %al
        repz cmpsb
        movsbl  %al, %eax
        seta    %dl
        sbbb    $0, %dl
        movsbl  %dl, %edx
        addl    %edx, %eax
        ret
        .cfi_endproc
.LFE0:
        .size   main, .-main
        .ident  "GCC: (Debian 8.3.0-6) 8.3.0"
        .section        .note.GNU-stack,"",@progbits
<<<<

Which seems to indicate that when comparing to constant strings every
function behaves similarly.

Francisco Olarte.
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Ranier Vilela-2
Em seg., 16 de nov. de 2020 às 09:00, Francisco Olarte <[hidden email]> escreveu:
Anyway, what I was going to send. Are you aware that many of these
micro optimizations are cargo cult from a bygone age of simpler
compilers?
Yes, there is an inheritance there.
But keep in mind, that Lua is compiled in a huge variety of environments and compilers.
And not everyone is lucky enough to have a super efficient compiler available.
In addition, small discrepancies can penalize some hotpath, even in a modern compiler.
Make it clear to the compiler that you can optimize, for sure, will generate better code.

regards,
Ranier Vilela
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Andrew Gierth
In reply to this post by Ranier Vilela-2
>>>>> "Ranier" == Ranier Vilela <[hidden email]> writes:

 Ranier> But if that is really the case, it is simple to resolve and
 Ranier> continue to avoid calling strlen over and over again.

strcmp doesn't call strlen (in reasonable implementations) and strncmp
isn't necessarily any faster. Trying to prefer strncmp over strcmp leads
to less obvious and more error-prone code.

 Ranier> -    lua_pushstring(L, "k");
 Ranier> +    lua_pushlstring(L, "k", 1);

 >> This really isn't likely to be worth it.

 Ranier> Standardization.

lua_pushliteral used to do this optimization, so if it _were_ worth it,
then it would be simple to just use lua_pushliteral in place of
lua_pushstring. But given that recent lua versions expand
lua_pushliteral as just lua_pushstring, it's clearly not worth it.

Your approach is strictly worse than either, by having the programmer
explicitly supply a value that the compiler or library can find out for
itself.

 Ranier> Strchr is it's more efficient than strcmp.

 >> It also does something completely different, which is not applicable
 >> here.

 Ranier> Basically, it does a search for '-',

No, the code in question is testing whether the argument (filename) is
_exactly equal to_ "-", not whether it _contains_ '-'.

--
Andrew.
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Ranier Vilela-2

Em seg., 16 de nov. de 2020 às 11:18, Andrew Gierth <[hidden email]> escreveu:
>>>>> "Ranier" == Ranier Vilela <[hidden email]> writes:

 Ranier> But if that is really the case, it is simple to resolve and
 Ranier> continue to avoid calling strlen over and over again.

strcmp doesn't call strlen (in reasonable implementations) and strncmp
isn't necessarily any faster.
False.
Strncmp always is faster.
Once strcmp, it will go to the end of the string, always.

Trying to prefer strncmp over strcmp leads
to less obvious and more error-prone code.
Good reason to go slow, like a turtle.


 Ranier> -    lua_pushstring(L, "k");
 Ranier> +    lua_pushlstring(L, "k", 1);

 >> This really isn't likely to be worth it.

 Ranier> Standardization.

lua_pushliteral used to do this optimization, so if it _were_ worth it,
then it would be simple to just use lua_pushliteral in place of
lua_pushstring. But given that recent lua versions expand
lua_pushliteral as just lua_pushstring, it's clearly not worth it.

Your approach is strictly worse than either, by having the programmer
explicitly supply a value that the compiler or library can find out for
itself.
So, why did they implement lua_pushlstring?
Perhaps not to be used.


 Ranier> Strchr is it's more efficient than strcmp.

 >> It also does something completely different, which is not applicable
 >> here.

 Ranier> Basically, it does a search for '-',

No, the code in question is testing whether the argument (filename) is
_exactly equal to_ "-", not whether it _contains_ '-'.
Maybe. But I want to hear the Lua Team.

Ranier Vilela
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Andre Leiradella-2


On 16/11/20 15:35, Ranier Vilela wrote:

False.
Strncmp always is faster.
Once strcmp, it will go to the end of the string, always.
Not false, since strcmp only goes as far as the first NUL is found in one of the strings.

Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Andre Leiradella-2
In reply to this post by Ranier Vilela-2

Sorry, hit send too fast.

On 16/11/20 15:35, Ranier Vilela wrote:


No, the code in question is testing whether the argument (filename) is
_exactly equal to_ "-", not whether it _contains_ '-'.
Maybe. But I want to hear the Lua Team.


This is not a maybe, your change does not do the same thing and is wrong, you don't need the Lua team to tell you that, you can read the man pages of these functions or believe what everyone here is telling you.


Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Ranier Vilela-2
In reply to this post by Andre Leiradella-2
Em seg., 16 de nov. de 2020 às 12:41, Andre Leiradella <[hidden email]> escreveu:


On 16/11/20 15:35, Ranier Vilela wrote:

False.
Strncmp always is faster.
Once strcmp, it will go to the end of the string, always.
Not false, since strcmp only goes as far as the first NUL is found in one of the strings.
Maybe you can then explain why they implemented strncmp, if strcmp solves everything and is as fast and better than strncmp?

/* strncmp example */
#include <stdio.h>
#include <string.h>

int main ()
{
  char str[][5] = { "R2D2" , "C3PO" , "R2A6" };
  int n;
  puts ("Looking for R2 astromech droids...");
  for (n=0 ; n<3 ; n++)
    if (strncmp (str[n],"R2xx",2) == 0)
    {
      printf ("found %s\n",str[n]);
    }
  return 0;
}
Why would they take the trouble?

Ranier Vilela
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Julien Cugnière
In reply to this post by Ranier Vilela-2
Le lun. 16 nov. 2020 à 16:36, Ranier Vilela <[hidden email]> a écrit :
> Em seg., 16 de nov. de 2020 às 11:18, Andrew Gierth <[hidden email]> escreveu:
>> strcmp doesn't call strlen (in reasonable implementations) and strncmp
>> isn't necessarily any faster.
>
> False.
> Strncmp always is faster.
> Once strcmp, it will go to the end of the string, always.

Look at strcmp in the GNU C Library (glibc, used in linux systems):
https://code.woboq.org/userspace/glibc/string/strcmp.c.html

Like I explained to you earlier, strcmp stops on the first difference,
or the first \0 (end of the shorter of the two string), whichever
comes first.

So in the instance you propose changing, strncmp will *not* reduce the
number of characters compared.

strcmp("a long time ago...", "method") compares a single character, and stops.
strcmp("methodxxxxxxxxxxxx", "method") compares 7 characters, and stops.
Reply | Threaded
Open this post in threaded view
|

Re: Lua 5.4.2 New assorted suggestions

Andre Leiradella-2
In reply to this post by Ranier Vilela-2


On 16/11/20 15:49, Ranier Vilela wrote:
Maybe you can then explain why they implemented strncmp, if strcmp solves everything and is as fast and better than strncmp?
Because they solve different problems.

12