* [9front] [PATCH] acme: limit reads of the event file to one read.
@ 2021-06-11 20:00 james palmer
2021-06-12 11:43 ` james palmer
2021-06-12 14:10 ` ori
0 siblings, 2 replies; 4+ messages in thread
From: james palmer @ 2021-06-11 20:00 UTC (permalink / raw)
To: 9front
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 <foura@biobuf.link>
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; i<w->nincl; 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;
+ 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [9front] [PATCH] acme: limit reads of the event file to one read.
2021-06-11 20:00 [9front] [PATCH] acme: limit reads of the event file to one read james palmer
@ 2021-06-12 11:43 ` james palmer
2021-06-12 14:22 ` james palmer
2021-06-12 14:10 ` ori
1 sibling, 1 reply; 4+ messages in thread
From: james palmer @ 2021-06-12 11:43 UTC (permalink / raw)
To: 9front
caught a small error in this,
winclose() was calling free(cur); then free(cur->buf);
new patch preventing a crash on some window closing follows.
thanks,
- james
From 57ebf7a69f14fa52cffea12883c2c7b5257a1bdd
From: james palmer <foura@biobuf.link>
Date: Sat, 12 Jun 2021 11:40:03 +0000
Subject: [PATCH] acme: limit event file reads to one event
this makes writing clients easier because events
can be pasted into a sufficiently large buffer and parsed
rather than read one byte at a time.
---
diff 09b0eb0d1ac7a66569263d2c48aea96e524ccebe 57ebf7a69f14fa52cffea12883c2c7b5257a1bdd
--- a/sys/src/cmd/acme/dat.h Tue Jun 8 21:13:57 2021
+++ b/sys/src/cmd/acme/dat.h Sat Jun 12 12:40:03 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 Tue Jun 8 21:13:57 2021
+++ b/sys/src/cmd/acme/wind.c Sat Jun 12 12:40:03 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; i<w->nincl; i++)
free(w->incl[i]);
free(w->incl);
- free(w->events);
+
+ cur = w->evhead;
+ while(cur) {
+ next = cur->next;
+ free(cur->buf);
+ free(cur);
+ 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 Tue Jun 8 21:13:57 2021
+++ b/sys/src/cmd/acme/xfid.c Sat Jun 12 12:40:03 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;
+ 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [9front] [PATCH] acme: limit reads of the event file to one read.
2021-06-11 20:00 [9front] [PATCH] acme: limit reads of the event file to one read james palmer
2021-06-12 11:43 ` james palmer
@ 2021-06-12 14:10 ` ori
1 sibling, 0 replies; 4+ messages in thread
From: ori @ 2021-06-12 14:10 UTC (permalink / raw)
To: 9front
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 <james@biobuf.link>:
> 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 <foura@biobuf.link>
> 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; i<w->nincl; 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [9front] [PATCH] acme: limit reads of the event file to one read.
2021-06-12 11:43 ` james palmer
@ 2021-06-12 14:22 ` james palmer
0 siblings, 0 replies; 4+ messages in thread
From: james palmer @ 2021-06-12 14:22 UTC (permalink / raw)
To: 9front mailing list
don't merge, broken
- james
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-14 20:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11 20:00 [9front] [PATCH] acme: limit reads of the event file to one read james palmer
2021-06-12 11:43 ` james palmer
2021-06-12 14:22 ` james palmer
2021-06-12 14:10 ` ori
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).