On Mon, Aug 14, 2017 at 10:26:48PM -0400, Jeremie Courreges-Anglas wrote:
>
> So I tinkered with the way ksh(1) tracks memory allocation, trying to
> make it faster in the general case. One approach used a RB tree,
> I wrote since a simple hash table implementation which seems to work
> rather well.
>
> But the actual problem I'd first like to solve is a corner case. I use
> HISTSIZE=20000, and when the actual line count in my histfile approaches
> 25000 (1.25 * HISTSIZE), ksh(1) has a hard time handling it. The main
> reason is that it calls afree() ~5000 times in a loop, with afree()
> traversing the APERM freelist, which contains >20000 elements. This is
> expensive.
>
> For history lines, we don't actually need to keep track of allocations
> using an area, history lines are private to history.c and no gc/whatever
> is needed there. So here's a diff that just uses strdup(3)/free(3).
>
> Comments? ok?
I was able to reproduce the problem with a HISTSIZE of 100000 which at 125000
entries rendered my system unusable. With the patch I am running fine with a
HISTSIZE of 120000 and have come back several times after hitting the 1.25x
threshold.
Regression tests pass.
Rob
> Index: history.c
> ===================================================================
> RCS file: /d/cvs/src/bin/ksh/history.c,v
> retrieving revision 1.64
> diff -u -p -p -u -r1.64 history.c
> --- history.c 11 Aug 2017 19:37:58 -0000 1.64
> +++ history.c 15 Aug 2017 01:14:58 -0000
> @@ -428,7 +428,7 @@ histbackup(void)
>
> if (histptr >= history && last_line != hist_source->line) {
> hist_source->line--;
> - afree(*histptr, APERM);
> + free(*histptr);
> histptr--;
> last_line = hist_source->line;
> }
> @@ -613,14 +613,15 @@ histsave(int lno, const char *cmd, int d
> #endif
> }
>
> - c = str_save(cmd, APERM);
> + if ((c = strdup(cmd)) == NULL)
> + internal_errorf(1, "unable to allocate memory");
> if ((cp = strrchr(c, '\n')) != NULL)
> *cp = '\0';
>
> if (histptr < history + histsize - 1)
> histptr++;
> else { /* remove oldest command */
> - afree(*history, APERM);
> + free(*history);
> memmove(history, history + 1,
> (histsize - 1) * sizeof(*history));
> }
>
>
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
>
