Re: Lua 5.4.2 New assorted suggestions (Part II)

[Kyle Evans <kevans@...>]

On Mon, Nov 23, 2020 at 10:20 AM Ranier Vilela <ranier.vf@gmail.com> wrote:
>
> Em seg., 23 de nov. de 2020 às 11:02, Kyle Evans <kevans@freebsd.org> escreveu:
>>
>> On Mon, Nov 23, 2020 at 5:38 AM Ranier Vilela <ranier.vf@gmail.com> wrote:
>> >
>> > Em dom., 22 de nov. de 2020 às 18:13, Kyle Evans <kevans@freebsd.org> escreveu:
>> >> Note that any optimizing compiler worth its salt should be able
>> >> to identify that it can eliminate the strlen() call at the call sites
>> >> where it's a literal, providing a less-redundant interface that's
>> >> "optimal" (not that I think it's necessarily
>> >> worth it).
>> >
>> > I come from the old school, from the days of the MSDOS.
>> > From my first C course, one of the lessons was: don't leave anything behind.
>> > Indeed, compilers have evolved a lot.
>> > But the one who knows the code and the data best is the programmer, not the compiler.
>> > Then give permission to optimize and the compiler will do a better job.
>> > It is the same principle of using const, many do not, because the code is less readable.
>> >
>>
>> Sure, I'll bow out here.
>
> I would like, at least, once again, your opinion, in this example below, if you don't mind.
> When the programmer knows the data, better than the compiler.
>
> LUA API luaS_newshortstr
> 1. Avoid "if test" when the size of string is < LUAI_MAXSHORTLEN.
>

The patch below seems to be truncated in some sense, because it
doesn't seem to do what's just been described. Notably, the
description leads me to believe that it's supposed to eliminate some
branching, but that doesn't seem to have happened.

Btw, I think we should take this off-list if you'd like my input,
rather than adding more tangentially related e-mail during an ongoing
release cycle. Not to say that there's no educational value in this
discussion, but I suspect it would be greatly appreciated to converge
on some agreeable set of changes (or maybe even no changes, if it
works out that way) before revisiting the topic on-list.

>
> lstring.c
> @@ -185,15 +185,15 @@ static void growstrtab (lua_State *L, stringtable *tb) {
>  /*
>  ** Checks whether short string exists and reuses it or creates a new one.
>  */
> -static TString *internshrstr (lua_State *L, const char *str, size_t l) {
> +TString *luaS_newshortstr (lua_State *L, const char * const str, size_t l) {
>
> @@ -218,9 +217,9 @@ static TString *internshrstr (lua_State *L, const char *str, size_t l) {
>  /*
>  ** new string (with explicit length)
>  */
> -    return internshrstr(L, str, l);
> +    return luaS_newshortstr(L, str, l);
>
> lapi.c
> @@ -1256,7 +1256,7 @@ LUA_API void lua_concat (lua_State *L, int n) {
>    if (n > 0)
>      luaV_concat(L, n);
>    else {  /* nothing to concatenate */
> -    setsvalue2s(L, L->top, luaS_newlstr(L, "", 0));  /* push empty string */
> +    setsvalue2s(L, L->top, luaS_newshortstr(L, "", 0));  /* push empty string */
>
>
> +++ b/lobject.c
> @@ -372,9 +372,9 @@ static int tostringbuff (TValue *obj, char *buff) {
>  ** Convert a number object to a Lua string, replacing the value at 'obj'
>  */
>  void luaO_tostring (lua_State *L, TValue *obj) {
> -  char buff[MAXNUMBER2STR];
> +  char buff[LUAI_MAXSHORTLEN];
>    int len = tostringbuff(obj, buff);
> -  setsvalue(L, obj, luaS_newlstr(L, buff, len));
> +  setsvalue(L, obj, luaS_newshortstr(L, buff, len));
>
> +++ b/lvm.c
>  /* copy strings in stack from top - n up to top - 1 to buffer */
>  static void copy2buff (StkId top, int n, char *buff) {
> @@ -661,7 +661,7 @@ void luaV_concat (lua_State *L, int total) {
>        if (tl <= LUAI_MAXSHORTLEN) {  /* is result a short string? */
>          char buff[LUAI_MAXSHORTLEN];
>          copy2buff(top, n, buff);  /* copy strings to buffer */
> -        ts = luaS_newlstr(L, buff, tl);
> +        ts = luaS_newshortstr(L, buff, tl);
>
> diff --git a/lundump.c b/lundump.c
> index 5aa55c44..b763d0fa 100644
> --- a/lundump.c
> +++ b/lundump.c
> @@ -116,7 +116,7 @@ static TString *loadStringN (LoadState *S, Proto *p) {
>    else if (--size <= LUAI_MAXSHORTLEN) {  /* short string? */
>      char buff[LUAI_MAXSHORTLEN];
>      loadVector(S, buff, size);  /* load string into buffer */
> -    ts = luaS_newlstr(L, buff, size);  /* create string */
> +    ts = luaS_newshortstr(L, buff, size);  /* create string */
>    }
>    else {  /* long string */
>      ts = luaS_createlngstrobj(L, size);  /* create string */
>
> What do you think?
>
> regards,
> Ranier Vilela