Re: Possible huge setarrvalue optimization

[Daniel Shahaf <d.s@daniel.shahaf.name>]

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

Bart Schaefer wrote on Sun, Nov 20, 2016 at 09:41:48 -0800:
> This and the proposed getstr optimization both make me nervous.  I know
> Sebastian is anxious to have them appear in the next release, but it feels
> and if we should have more time using them in dev branches.

I assume we can go ahead now.

Here's a revised patch based on my review upthread:

diff --git a/Src/params.c b/Src/params.c
index 82554a7..c4dad8f 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2708,6 +2708,23 @@ setarrvalue(Value v, char **val)
 	    post_assignment_length += pre_assignment_length - v->end;
 	}
 
+	if (pre_assignment_length == post_assignment_length
+	    && v->pm->gsu.a->setfn == arrsetfn
+	    /* ... and isn't something that arrsetfn() treats specially */
+	    && 0 == (v->pm->node.flags & (PM_SPECIAL|PM_UNIQUE))
+	    && NULL == v->pm->ename)
+    {
+	/* v->start is 0-based */
+	p = old + v->start;
+	for (r = val; *r;) {
+	    /* Free previous string */
+	    zsfree(*p);
+	    /* Give away ownership of the string */
+	    *p++ = *r++;
+	}
+    }
+	else
+    {
 	p = new = (char **) zalloc(sizeof(char *)
 		                   * (post_assignment_length + 1));
 
@@ -2726,6 +2743,7 @@ setarrvalue(Value v, char **val)
 	       post_assignment_length, (unsigned long)(p - new));
 
 	v->pm->gsu.a->setfn(v->pm, new);
+    }
 
         /* Ownership of all strings has been
          * given away, can plainly free */
@@ -3485,6 +3503,8 @@ arrsetfn(Param pm, char **x)
     /* Arrays tied to colon-arrays may need to fix the environment */
     if (pm->ename && x)
 	arrfixenv(pm->ename, x);
+    /* If you extend this function, update the list of conditions in
+     * setarrvalue(). */
 }
 
 /* Function to get value of an association parameter */

If no objections, I'll reindent and push.

Cheers,

Daniel

[-- Attachment #2: 0001-39996-Optimize-setarrvalue.patch --]
[-- Type: text/x-diff, Size: 1698 bytes --]

>From b4870d1cab9db235860adcadd821069b2e2162e9 Mon Sep 17 00:00:00 2001
From: Sebastian Gniazdowski <psprint@fastmail.com>
Date: Tue, 22 Nov 2016 09:26:46 +0000
Subject: [PATCH] AFTER RELEASE: 39996: Optimize setarrvalue().

---
 Src/params.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Src/params.c b/Src/params.c
index 82554a7..c4dad8f 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2708,6 +2708,23 @@ setarrvalue(Value v, char **val)
 	    post_assignment_length += pre_assignment_length - v->end;
 	}
 
+	if (pre_assignment_length == post_assignment_length
+	    && v->pm->gsu.a->setfn == arrsetfn
+	    /* ... and isn't something that arrsetfn() treats specially */
+	    && 0 == (v->pm->node.flags & (PM_SPECIAL|PM_UNIQUE))
+	    && NULL == v->pm->ename)
+    {
+	/* v->start is 0-based */
+	p = old + v->start;
+	for (r = val; *r;) {
+	    /* Free previous string */
+	    zsfree(*p);
+	    /* Give away ownership of the string */
+	    *p++ = *r++;
+	}
+    }
+	else
+    {
 	p = new = (char **) zalloc(sizeof(char *)
 		                   * (post_assignment_length + 1));
 
@@ -2726,6 +2743,7 @@ setarrvalue(Value v, char **val)
 	       post_assignment_length, (unsigned long)(p - new));
 
 	v->pm->gsu.a->setfn(v->pm, new);
+    }
 
         /* Ownership of all strings has been
          * given away, can plainly free */
@@ -3485,6 +3503,8 @@ arrsetfn(Param pm, char **x)
     /* Arrays tied to colon-arrays may need to fix the environment */
     if (pm->ename && x)
 	arrfixenv(pm->ename, x);
+    /* If you extend this function, update the list of conditions in
+     * setarrvalue(). */
 }
 
 /* Function to get value of an association parameter */