From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=0.0 required=5.0 tests=none autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 9582 invoked from network); 14 Jun 2021 20:04:46 -0000 Received: from 1ess.inri.net (216.126.196.35) by inbox.vuxu.org with ESMTPUTF8; 14 Jun 2021 20:04:46 -0000 Received: from mimir.eigenstate.org ([206.124.132.107]) by 1ess; Mon Jun 14 15:57:04 -0400 2021 Received: from abbatoir.myfiosgateway.com (pool-74-108-56-225.nycmny.fios.verizon.net [74.108.56.225]) by mimir.eigenstate.org (OpenSMTPD) with ESMTPSA id e2996a5a (TLSv1.2:ECDHE-RSA-AES256-SHA:256:NO) for <9front@9front.org>; Sat, 12 Jun 2021 07:10:14 -0700 (PDT) Message-ID: <9CB57FC9074A1CEF5875C545BC8E762D@eigenstate.org> To: 9front@9front.org Date: Sat, 12 Jun 2021 10:10:13 -0400 From: ori@eigenstate.org In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit List-ID: <9front.9front.org> List-Help: X-Glyph: ➈ X-Bullshit: asynchronous framework storage GPU frontend Subject: Re: [9front] [PATCH] acme: limit reads of the event file to one read. Reply-To: 9front@9front.org Precedence: bulk Definitely makes it easier to write acme clients, I'm in favor. Style nitpicks: if(foo) {, if (foo) { should be: if(foo){ It's also worth asking if we should unfuck the message format, which would be an incompatible change. 1. Always return exactly one message set, rather than splitting across reads 2. Counting the message size in bytes, so you can read and copy the required amount, rather than trying to figure out the byte count from the rune count 3. Maybe do something about the difficult-to-interpret continuation messages; eg, flatten them into one event, and put the required information into the one event. I'd be ok with it, but I'm not a heavy acme user, and I don't know how many acme users use external tools that'd need to be fixed up. Also, we've been more or less treating plan9port as acme upstream, not sure if we'd want to diverge. Quoth james palmer : > this patch limits acme to one message per read of the event file. > this makes writing clients easier because events can be read into a > sufficiently large buffer and parsed instead of read one byte at a time. > > git/import-able patch follows. > > - james > > From 189797ed9098d5f6cc06cda299f1941dc07dee08 > From: james palmer > Date: Fri, 11 Jun 2021 19:41:38 +0000 > Subject: [PATCH] acme: limit reads of the events file to one event for easier parsing. > > --- > diff 3ec67240f59d1e68b8d7fcb30fa17fe87f317bff 189797ed9098d5f6cc06cda299f1941dc07dee08 > --- a/sys/src/cmd/acme/dat.h Mon Jun 7 10:13:51 2021 > +++ b/sys/src/cmd/acme/dat.h Fri Jun 11 20:41:38 2021 > @@ -44,6 +44,7 @@ > typedef struct Fid Fid; > typedef struct File File; > typedef struct Elog Elog; > +typedef struct Event Event; > typedef struct Mntdir Mntdir; > typedef struct Range Range; > typedef struct Rangeset Rangeset; > @@ -256,8 +257,8 @@ > int rdselfd; > Column *col; > Xfid *eventx; > - char *events; > - int nevents; > + Event *evhead; > + Event *evtail; > int owner; > int maxlines; > Dirlist **dlp; > @@ -278,6 +279,14 @@ > int tagexpand; > int taglines; > Rectangle tagtop; > + int deleted; > +}; > + > +struct Event { > + char *buf; > + int sz; > + int offset; > + Event *next; > }; > > void wininit(Window*, Window*, Rectangle); > --- a/sys/src/cmd/acme/wind.c Mon Jun 7 10:13:51 2021 > +++ b/sys/src/cmd/acme/wind.c Fri Jun 11 20:41:38 2021 > @@ -22,6 +22,9 @@ > Rune *rp; > int nc, i; > > + w->deleted = 0; > + w->evhead = nil; > + w->evtail = nil; > w->tag.w = w; > w->taglines = 1; > w->tagexpand = TRUE; > @@ -311,6 +314,7 @@ > winclose(Window *w) > { > int i; > + Event *cur, *next; > > if(decref(w) == 0){ > xfidlog(w, "del"); > @@ -322,7 +326,15 @@ > for(i=0; inincl; i++) > free(w->incl[i]); > free(w->incl); > - free(w->events); > + > + cur = w->evhead; > + while(cur) { > + next = cur->next; > + free(cur); > + free(cur->buf); > + cur = next; > + } > + > free(w); > } > } > @@ -331,12 +343,21 @@ > windelete(Window *w) > { > Xfid *x; > + Event *cur, *next; > > x = w->eventx; > if(x){ > - w->nevents = 0; > - free(w->events); > - w->events = nil; > + cur = w->evhead; > + while(cur) { > + next = cur->next; > + free(cur->buf); > + free(cur); > + cur = next; > + } > + > + w->deleted = 1; > + w->evhead = nil; > + w->evtail = nil; > w->eventx = nil; > sendp(x->c, nil); /* wake him up */ > } > @@ -669,26 +690,42 @@ > void > winevent(Window *w, char *fmt, ...) > { > - int n; > - char *b; > Xfid *x; > + char *buf; > va_list arg; > + Event *ev; > > if(w->nopen[QWevent] == 0) > return; > if(w->owner == 0) > - error("no window owner"); > + error("no window owner"); > + > + ev = emalloc(sizeof(Event)); > + ev->next = nil; > + > va_start(arg, fmt); > - b = vsmprint(fmt, arg); > + buf = vsmprint(fmt, arg); > va_end(arg); > - if(b == nil) > + > + if(buf == nil) > error("vsmprint failed"); > - n = strlen(b); > - w->events = erealloc(w->events, w->nevents+1+n); > - w->events[w->nevents++] = w->owner; > - memmove(w->events+w->nevents, b, n); > - free(b); > - w->nevents += n; > + > + ev->buf = smprint("%c%s", w->owner, buf); > + if(!ev->buf) > + error("smprint failed"); > + free(buf); > + > + ev->offset = 0; > + ev->sz = strlen(ev->buf); > + > + if(w->evhead) { > + w->evtail->next = ev; > + w->evtail = ev; > + } else { > + w->evhead = ev; > + w->evtail = ev; > + } > + > x = w->eventx; > if(x){ > w->eventx = nil; > --- a/sys/src/cmd/acme/xfid.c Mon Jun 7 10:13:51 2021 > +++ b/sys/src/cmd/acme/xfid.c Fri Jun 11 20:41:38 2021 > @@ -991,34 +991,45 @@ > xfideventread(Xfid *x, Window *w) > { > Fcall fc; > - char *b; > - int i, n; > + Event *ev; > + int bufsz; > + char *buf; > > - i = 0; > x->flushed = FALSE; > - while(w->nevents == 0){ > - if(i){ > - if(!x->flushed) > - respond(x, &fc, "window shut down"); > + while(!w->evhead){ > + if(w->deleted) { > + respond(x, &fc, "window shut down"); > return; > } > + > w->eventx = x; > winunlock(w); > recvp(x->c); > winlock(w, 'F'); > - i++; > } > - > - n = w->nevents; > - if(n > x->count) > - n = x->count; > - fc.count = n; > - fc.data = w->events; > + > + ev = w->evhead; > + bufsz = ev->sz - ev->offset < x->count > + ? ev->sz - ev->offset : x->count; there's a 'min()' in util.c > + buf = emalloc(bufsz); > + > + memcpy(buf, ev->buf+ev->offset, bufsz); > + ev->offset += bufsz; > + > + if(ev->offset == bufsz) { > + w->evhead = ev->next; > + free(ev->buf); > + free(ev); > + > + if(!w->evhead) > + w->evtail = nil; > + } > + > + > + fc.count = bufsz; > + fc.data = buf; > respond(x, &fc, nil); > - b = w->events; > - w->events = estrdup(w->events+n); > - free(b); > - w->nevents -= n; > + free(buf); > } > > void >