From: Peter Stephenson <p.w.stephenson@ntlworld.com>
To: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: PATH: autoload with explicit path
Date: Wed, 11 Jan 2017 20:51:22 +0000 [thread overview]
Message-ID: <20170111205122.2c47e89f@ntlworld.com> (raw)
In-Reply-To: <20170111114232.04eedd08@pwslap01u.europe.root.pri>
On Wed, 11 Jan 2017 11:42:32 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> The main disadvantage is each function record now contains the name of
> the directory, so this takes extra memory --- it looks like a couple of
> dozen k in the case of my function directory which is quite full. This
> could be optimised with indirection but I'm not sure it's worth the work.
This seemed like too good an idea not to follow up. Now "autoload
/blah/blah/*" is as efficient as the old way of doing it, too.
With a bit of work this can be expanded to paths for already loaded
functions, and possibly elsewhere though I think the pickings are
a lot smaller.
pws
diff --git a/Src/builtin.c b/Src/builtin.c
index b986dd8..0ed081a 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2957,13 +2957,13 @@ check_autoload(Shfunc shf, char *name, Options ops, int func)
}
}
if (getfpfunc(shf->node.nam, NULL, &dir_path, NULL, 1)) {
- zsfree(shf->filename);
+ dircache_set(&shf->filename, NULL);
if (*dir_path != '/') {
dir_path = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP),
"/", dir_path);
dir_path = xsymlink(dir_path, 1);
}
- shf->filename = ztrdup(dir_path);
+ dircache_set(&shf->filename, dir_path);
return 0;
}
if (OPT_ISSET(ops,'R')) {
@@ -3026,8 +3026,8 @@ add_autoload_function(Shfunc shf, char *funcname)
*nam++ = '\0';
dir = funcname;
}
- zsfree(shf->filename);
- shf->filename = ztrdup(dir);
+ dircache_set(&shf->filename, NULL);
+ dircache_set(&shf->filename, dir);
shfunctab->addnode(shfunctab, ztrdup(nam), shf);
} else {
shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
@@ -3276,8 +3276,8 @@ bin_functions(char *name, char **argv, Options ops, int func)
shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
}
if (*argv) {
- zsfree(shf->filename);
- shf->filename = ztrdup(*argv);
+ dircache_set(&shf->filename, NULL);
+ dircache_set(&shf->filename, *argv);
}
shf->node.flags = on;
ret = eval_autoload(shf, funcname, ops, func);
diff --git a/Src/exec.c b/Src/exec.c
index 7bec7ce..10a5249 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4902,6 +4902,7 @@ execfuncdef(Estate state, Eprog redir_prog)
shf = (Shfunc) zalloc(sizeof(*shf));
shf->funcdef = prog;
shf->node.flags = 0;
+ /* No dircache here, not a directory */
shf->filename = ztrdup(scriptfilename);
shf->lineno = lineno;
/*
@@ -4934,7 +4935,7 @@ execfuncdef(Estate state, Eprog redir_prog)
freeeprog(shf->funcdef);
if (shf->redir) /* shouldn't be */
freeeprog(shf->redir);
- zsfree(shf->filename);
+ dircache_set(&shf->filename, NULL);
zfree(shf, sizeof(*shf));
state->pc = end;
return 1;
@@ -4965,7 +4966,7 @@ execfuncdef(Estate state, Eprog redir_prog)
freeeprog(shf->funcdef);
if (shf->redir) /* shouldn't be */
freeeprog(shf->redir);
- zsfree(shf->filename);
+ dircache_set(&shf->filename, NULL);
zfree(shf, sizeof(*shf));
break;
} else {
@@ -4974,7 +4975,7 @@ execfuncdef(Estate state, Eprog redir_prog)
(signum = getsignum(s + 4)) != -1) {
if (settrap(signum, NULL, ZSIG_FUNC)) {
freeeprog(shf->funcdef);
- zsfree(shf->filename);
+ dircache_set(&shf->filename, NULL);
zfree(shf, sizeof(*shf));
state->pc = end;
return 1;
@@ -5204,7 +5205,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
else
shf->funcdef = dupeprog(prog, 0);
shf->node.flags &= ~PM_UNDEFINED;
- zsfree(shf->filename);
+ dircache_set(&shf->filename, NULL);
+ /* Full filename, don't use dircache */
shf->filename = fname;
} else {
VARARR(char, n, strlen(shf->node.nam) + 1);
@@ -5228,7 +5230,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
else
shf->funcdef = dupeprog(stripkshdef(prog, shf->node.nam), 0);
shf->node.flags &= ~PM_UNDEFINED;
- zsfree(shf->filename);
+ dircache_set(&shf->filename, NULL);
+ /* Full filename, don't use dircache */
shf->filename = fname;
}
popheap();
@@ -5619,6 +5622,8 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name)
* Search fpath for an undefined function. Finds the file, and returns the
* list of its contents.
*
+ * If test is 0, load the function; *fname is set to zalloc'ed location.
+ *
* If test_only is 1, don't load function, just test for it:
* - Non-null return means function was found
* - *fname points to path at which found (not duplicated)
diff --git a/Src/hashtable.c b/Src/hashtable.c
index 2a8b585..110cea6 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -889,7 +889,7 @@ freeshfuncnode(HashNode hn)
freeeprog(shf->funcdef);
if (shf->redir)
freeeprog(shf->redir);
- zsfree(shf->filename);
+ dircache_set(&shf->filename, NULL);
if (shf->sticky) {
if (shf->sticky->n_on_opts)
zfree(shf->sticky->on_opts,
@@ -1442,3 +1442,134 @@ freehistdata(Histent he, int unlink)
}
}
}
+
+
+/***********************************************************************
+ * Directory name cache mechanism
+ *
+ * The idea of this is that there are various shell structures,
+ * notably functions, that record the directories with which they
+ * are associated. Rather than store the full string each time,
+ * we store a pointer to the same location and count the references.
+ * This is optimised so that retrieval is quick at the expense of
+ * searching the list when setting up the structure, which is a much
+ * rarer operation.
+ *
+ * There is nothing special about the fact that the strings are
+ * directories, except for the assumptions for efficiency that many
+ * structures will point to the same one, and that there are not too
+ * many different directories associated with the shell.
+ **********************************************************************/
+
+struct dircache_entry
+{
+ /* Name of directory in cache */
+ char *name;
+ /* Number of references to it */
+ int refs;
+};
+
+/*
+ * dircache is the cache, of length dircache_size.
+ * dircache_lastentry is the last entry used, an optimisation
+ * for multiple references to the same directory, e.g
+ * "autoload /blah/blah/\*".
+ */
+struct dircache_entry *dircache, *dircache_lastentry;
+int dircache_size;
+
+/*
+ * Set *name to point to a cached version of value.
+ * value is copied so may come from any source.
+ *
+ * If value is NULL, look for the existing value of *name (safe if this
+ * too is NULL) and remove a reference to it from the cache. If it's
+ * not found in the cache, it's assumed to be an allocated string and
+ * freed --- this currently occurs for a shell function that's been
+ * loaded as the filename is now a full path, not just a directory,
+ * though we may one day optimise this to a cached directory plus a
+ * name, too. Note --- the function does *not* otherwise check
+ * if *name points to something already cached, so this is
+ * necessary any time *name may already be in the cache.
+ */
+
+/**/
+mod_export void
+dircache_set(char **name, char *value)
+{
+ struct dircache_entry *dcptr, *dcnew;
+
+ if (!value) {
+ if (!dircache_size || !*name)
+ return;
+
+ for (dcptr = dircache; dcptr < dircache + dircache_size; dcptr++)
+ {
+ /* Must be a pointer much, not a string match */
+ if (*name == dcptr->name)
+ {
+ --dcptr->refs;
+ if (!dcptr->refs) {
+ ptrdiff_t ind = dcptr - dircache;
+ zsfree(dcptr->name);
+ --dircache_size;
+
+ if (!dircache_size) {
+ zfree(dircache, sizeof(*dircache));
+ dircache = NULL;
+ return;
+ }
+ dcnew = (struct dircache_entry *)
+ zalloc(dircache_size * sizeof(*dcnew));
+ if (ind)
+ memcpy(dcnew, dircache, ind * sizeof(*dcnew));
+ if (ind < dircache_size)
+ memcpy(dcnew + ind, dcptr + 1,
+ (dircache_size - ind) * sizeof(*dcnew));
+ zfree(dircache, (dircache_size+1)*sizeof(*dcnew));
+ dircache = dcnew;
+ dircache_lastentry = NULL;
+ }
+ *name = NULL;
+ return;
+ }
+ }
+ zsfree(*name);
+ *name = NULL;
+ } else {
+ /*
+ * We'll maintain the cache at exactly the right size rather
+ * than overallocating. The rationale here is that typically
+ * we'll get a lot of functions in a small number of directories
+ * so the complexity overhead of maintaining a separate count
+ * isn't really matched by the efficiency gain.
+ */
+ if (!dircache_size) {
+ dircache_size = 1;
+ dcptr = dircache =
+ (struct dircache_entry *)zalloc(sizeof(*dircache));
+ } else if (dircache_lastentry &&
+ !strcmp(value, dircache_lastentry->name)) {
+ *name = dircache_lastentry->name;
+ ++dircache_lastentry->refs;
+ return;
+ } else {
+ for (dcptr = dircache; dcptr < dircache + dircache_size; dcptr++)
+ {
+ if (!strcmp(value, dcptr->name)) {
+ *name = dcptr->name;
+ ++dcptr->refs;
+ return;
+ }
+ }
+ ++dircache_size;
+ dircache = (struct dircache_entry *)
+ zrealloc(dircache, sizeof(*dircache) * dircache_size);
+ dcptr = dircache + dircache_size - 1;
+ }
+ dcptr->name = ztrdup(value);
+ *name = dcptr->name;
+ dcptr->refs = 1;
+ dircache_lastentry = dcptr;
+ }
+}
diff --git a/Src/signals.c b/Src/signals.c
index 9e05add..2de5743 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -811,6 +811,7 @@ dosavetrap(int sig, int level)
newshf->node.nam = ztrdup(shf->node.nam);
newshf->node.flags = shf->node.flags;
newshf->funcdef = dupeprog(shf->funcdef, 0);
+ /* Assume this is a real filename, don't use dircache */
newshf->filename = ztrdup(shf->filename);
if (shf->sticky) {
newshf->sticky = sticky_emulation_dup(shf->sticky, 0);
next prev parent reply other threads:[~2017-01-11 20:51 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-11 22:18 Peter Stephenson
2016-12-12 16:05 ` Bart Schaefer
2016-12-12 16:31 ` Peter Stephenson
2016-12-12 18:09 ` Bart Schaefer
2017-01-10 19:31 ` Peter Stephenson
2017-01-11 11:42 ` Peter Stephenson
2017-01-11 20:51 ` Peter Stephenson [this message]
2017-01-12 20:42 ` Peter Stephenson
2017-01-13 18:04 ` Peter Stephenson
2017-01-16 10:37 ` Peter Stephenson
2017-01-16 15:04 ` Daniel Shahaf
2017-01-16 15:48 ` Peter Stephenson
2017-01-16 15:22 ` Bart Schaefer
2017-01-16 15:59 ` Peter Stephenson
[not found] ` <CAHYJk3SB1NDj6y5TRHHsAVsyjHfZQhTzMRzTR2c-SVEc9oAwzA@mail.gmail.com>
2017-01-24 11:10 ` Peter Stephenson
2017-01-11 21:13 ` Peter Stephenson
2017-01-17 18:36 ` PATCH: " Peter Stephenson
2017-01-17 22:17 ` Daniel Shahaf
2017-01-18 0:06 ` Bart Schaefer
2017-01-18 9:21 ` Peter Stephenson
2017-01-18 9:17 ` Peter Stephenson
2017-01-18 22:26 ` Bart Schaefer
2017-01-19 9:39 ` Peter Stephenson
2017-01-18 9:53 ` Peter Stephenson
[not found] <CGME20170127151334epcas2p4c32b57f69fcae22b40b309793eb8ceb6@epcas2p4.samsung.com>
2017-01-27 15:12 ` PATH: " Sebastian Gniazdowski
2017-01-27 16:24 ` Peter Stephenson
2017-01-27 18:40 ` Sebastian Gniazdowski
2017-01-27 18:44 ` Peter Stephenson
2017-01-27 19:00 ` Sebastian Gniazdowski
2017-01-28 18:05 ` Bart Schaefer
2017-01-28 19:12 ` Peter Stephenson
2017-01-28 19:45 ` Bart Schaefer
2017-01-28 19:56 ` Peter Stephenson
2017-01-28 20:37 ` Sebastian Gniazdowski
2017-01-29 12:27 ` Sebastian Gniazdowski
2017-01-29 16:11 ` Bart Schaefer
2017-01-29 17:32 ` Sebastian Gniazdowski
2017-01-29 18:37 ` Bart Schaefer
2017-01-29 21:53 ` Vin Shelton
2017-01-30 10:06 ` Peter Stephenson
2017-01-29 17:58 ` Peter Stephenson
2017-01-30 11:37 ` Sebastian Gniazdowski
2017-01-30 11:55 ` Peter Stephenson
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=20170111205122.2c47e89f@ntlworld.com \
--to=p.w.stephenson@ntlworld.com \
--cc=zsh-workers@zsh.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 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).