zsh-workers
 help / color / mirror / code / Atom feed
* A patch with hashtable optimization, which doesn't work
@ 2017-05-18 10:14 Sebastian Gniazdowski
  2017-05-18 12:16 ` Sebastian Gniazdowski
  0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Gniazdowski @ 2017-05-18 10:14 UTC (permalink / raw)
  To: zsh-workers

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

Hello,
it is really simple to keep collision lists sorted. However, I get error about typeset not being found. Debugged that typeset is inserted into builtintab. I am pretty exhausted, maybe someone will have a revelation on this. Debugged that typeset is being searched in:

exec.c:729 if ((cn = (Cmdnam) cmdnamtab->getnode(cmdnamtab, arg0))) {

at final stage, so it apparently misses builtintab search before. Below is the main idea, attached is full patch.

-       if (ht->cmpnodes(hp->nam, hn->nam) == 0) {
+        cres = ht->cmpnodes(hp->nam, hn->nam);
+       if ( cres == 0 ) {
            hq->next = hn;
            goto replacing;
-       }
+       } else if ( cres > 0 ) {
+            /* Pointer `hp` is at 'right' position - it points
+             * to a greater element than `hn`, so we should
+             * insert at one-before, that is after `hq` */
+            hq->next = hn;
+            hn->next = hp;
+            if (++ht->ct >= ht->hsize * 2 && !ht->scan)
+                expandhashtable(ht);
+            return NULL;
+        }

--
Sebastian Gniazdowski
psprint /at/ zdharma.org

[-- Attachment #2: hashopt.diff.txt --]
[-- Type: text/plain, Size: 3148 bytes --]

diff --git a/Src/hashtable.c b/Src/hashtable.c
index ba5faad..d820efd 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -162,6 +162,7 @@ addhashnode(HashTable ht, char *nam, void *nodeptr)
 HashNode
 addhashnode2(HashTable ht, char *nam, void *nodeptr)
 {
+    int cres;
     unsigned hashval;
     HashNode hn, hp, hq;
 
@@ -181,7 +182,8 @@ addhashnode2(HashTable ht, char *nam, void *nodeptr)
     }
 
     /* else check if the first node contains the same key */
-    if (ht->cmpnodes(hp->nam, hn->nam) == 0) {
+    cres = ht->cmpnodes(hp->nam, hn->nam);
+    if ( cres == 0 ) {
 	ht->nodes[hashval] = hn;
 	replacing:
 	hn->next = hp->next;
@@ -196,21 +198,39 @@ addhashnode2(HashTable ht, char *nam, void *nodeptr)
 		ht->scan->u.u = hn;
 	}
 	return hp;
+    } else if ( cres > 0 ) {
+        /* Insert in front of the list, because first
+         * element, nodes[hashval], is larger, so new
+         * element has to be before it */
+        hn->next = hp;
+        ht->nodes[hashval] = hn;
+	if (++ht->ct >= ht->hsize * 2 && !ht->scan)
+	    expandhashtable(ht);
     }
 
     /* else run through the list and check all the keys */
     hq = hp;
     hp = hp->next;
     for (; hp; hq = hp, hp = hp->next) {
-	if (ht->cmpnodes(hp->nam, hn->nam) == 0) {
+        cres = ht->cmpnodes(hp->nam, hn->nam);
+	if ( cres == 0 ) {
 	    hq->next = hn;
 	    goto replacing;
-	}
+	} else if ( cres > 0 ) {
+            /* Pointer `hp` is at 'right' position - it points
+             * to a greater element than `hn`, so we should
+             * insert at one-before, that is after `hq` */
+            hq->next = hn;
+            hn->next = hp;
+            if (++ht->ct >= ht->hsize * 2 && !ht->scan)
+                expandhashtable(ht);
+            return NULL;
+        }
     }
 
-    /* else just add it at the front of the list */
-    hn->next = ht->nodes[hashval];
-    ht->nodes[hashval] = hn;
+    /* else just add it at the end of the list */
+    hq->next = hn;
+    hn->next = NULL;
     if (++ht->ct >= ht->hsize * 2 && !ht->scan)
         expandhashtable(ht);
     return NULL;
@@ -225,17 +245,20 @@ addhashnode2(HashTable ht, char *nam, void *nodeptr)
 mod_export HashNode
 gethashnode(HashTable ht, const char *nam)
 {
+    int cres;
     unsigned hashval;
     HashNode hp;
 
     hashval = ht->hash(nam) % ht->hsize;
     for (hp = ht->nodes[hashval]; hp; hp = hp->next) {
-	if (ht->cmpnodes(hp->nam, nam) == 0) {
+        cres = ht->cmpnodes(hp->nam, nam);
+	if (cres == 0) {
 	    if (hp->flags & DISABLED)
 		return NULL;
 	    else
 		return hp;
-	}
+	} else if (cres > 0)
+            return NULL;
     }
     return NULL;
 }
@@ -249,13 +272,17 @@ gethashnode(HashTable ht, const char *nam)
 mod_export HashNode
 gethashnode2(HashTable ht, const char *nam)
 {
+    int cres;
     unsigned hashval;
     HashNode hp;
 
     hashval = ht->hash(nam) % ht->hsize;
     for (hp = ht->nodes[hashval]; hp; hp = hp->next) {
-	if (ht->cmpnodes(hp->nam, nam) == 0)
+        cres = ht->cmpnodes(hp->nam, nam);
+	if (cres == 0)
 	    return hp;
+        else if (cres > 0)
+            return NULL;
     }
     return NULL;
 }

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: Improving zcompdump (Re: A patch with hashtable optimization, which doesn't work)
@ 2017-06-03 14:11 Sebastian Gniazdowski
  2017-06-04  0:54 ` Bart Schaefer
  0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Gniazdowski @ 2017-06-03 14:11 UTC (permalink / raw)
  To: Bart Schaefer, zsh-workers

On 20 maja 2017 at 19:08:09, Bart Schaefer (schaefer@brasslantern.com) wrote:
> } Tried to optimize mkautofn, to speed up sourcing zcompdump.
>
> How much does zcompdump actually help? Have you compared startup with
> and without it?
>
> There's a bunch of stuff in .zcompdump. Have you investigated whether
> certain parts of it are slower than others?
>
> One lesson learned with Completion/Base/Utility/_store_cache is that
> parsing array assignments is expensive.

I've wrapped sourcing zcompdump in compinit this way:

      zmodload zsh/zprof
      () {
          builtin . "$_comp_dumpfile"
      }
      zprof | head -n 14

Then I tried with a) normal .zcompdump, and b) with modification – with _comps=( ), i.e. empty. Results seem to confirm what you said:

num  calls                time                       self            name
-----------------------------------------------------------------------------------
 1)    1          58,93    58,93  100,00%     58,93    58,93  100,00%  (anon)

vs.

 1)    1          12,81    12,81  100,00%     12,81    12,81  100,00%  (anon)

There's 58-12=46 ms to win, a significant value when thinking in terms of instant Zsh startup, which today is rather a melody of the past, with zsh-syntax-highlighting and zsh-autosuggestions overloading all $widgets entries during startup, in a loop.

I would go in direction of implementing new trivial parser that would read key-value pairs and put them to hash. It might even predict required size for 1562 _comps elements in the hash (it's x4 AFAIR, saw in addhashnode2), so that no expandhashtable() will be called. There would be .zcompdump_comps file with the pairs. Nothing will break, old .zcompdump will work.

--
Sebastian Gniazdowski
psprint /at/ zdharma.org


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-06-04  7:18 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-18 10:14 A patch with hashtable optimization, which doesn't work Sebastian Gniazdowski
2017-05-18 12:16 ` Sebastian Gniazdowski
2017-05-20  5:06   ` Sebastian Gniazdowski
2017-05-20 17:08     ` Improving zcompdump (Re: A patch with hashtable optimization, which doesn't work) Bart Schaefer
2017-06-03 14:11 Sebastian Gniazdowski
2017-06-04  0:54 ` Bart Schaefer
2017-06-04  7:18   ` Sebastian Gniazdowski

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).