* [GNU ELPA] New package: tam @ 2023-09-18 2:28 Lynn Winebarger 2023-09-18 9:37 ` Philip Kaludercic ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Lynn Winebarger @ 2023-09-18 2:28 UTC (permalink / raw) To: emacs-devel I'd like to submit "tam" (table allocation manager) for inclusion in GNU ELPA. The source is available at https://github.com/owinebar/emacs-table-allocation-manager tam provides functions to track the (manual) allocation of a fixed number of resources to specific objects. All the data structures of the table are fixed at initialization, so that adding or removing a particular object should not provoke a garbage collection in any obvious way. The allocation and deallocation are meant for use in process sentinels and the like. I have previously implemented some of the functionality of this package in a package async-job-queue on MELPA. However, since that work cannot be assigned to the FSF, I am writing a more general purpose async job scheduler from scratch. In particular, the scheduler will provide a facility for users to set a maximum number of async processes to run simultaneously, then manage the assignment of those process slots to jobs. The tam functions provide a generic way to manage that resource allocation in process sentinels. tam.el was created on September 15, 2023. ;;; tam.el --- Manage use of slots in a fixed size table -*- lexical-binding: t; -*- ;;; Commentary: ;; Table Allocation Manager ;; Provides an interface to managing the usage of the slots in a fixed size ;; table. All allocation is done during initialization to avoid triggering ;; garbage collection during allocation/free operations. ;; API: ;; ;; (tam-create-table N) => table of size N ;; (tam-table-fullp TABLE) => nil unless TABLE is full ;; (tam-table-emptyp TABLE) => nil unless TABLE is empty ;; (tam-table-size TABLE) => number of slots in TABLE ;; (tam-table-used TABLE) => number of slots of TABLE in use ;; (tam-table-get TABLE IDX) => contents of TABLE slot at index IDX ;; (tam-allocate TABLE OBJ) => ;; if not full, assigns OBJ to contents of a free slot in TABLE, ;; and returns the index of the slot ;; if full, returns nil ;; (tam-free TABLE INDEX) => ;; if slot at INDEX of TABLE is in use, move to the free list and ;; return the object formerly held by the slot ;; if slot is already free, signal an error ;; (tam-table-free-list TABLE) => list of free indices in TABLE ;; (tam-table-live-list TABLE) => list of in-use indices in TABLE Thanks, Lynn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-18 2:28 [GNU ELPA] New package: tam Lynn Winebarger @ 2023-09-18 9:37 ` Philip Kaludercic 2023-09-18 16:22 ` Lynn Winebarger 2023-09-18 19:26 ` Adam Porter 2023-09-20 16:06 ` Stefan Monnier 2 siblings, 1 reply; 20+ messages in thread From: Philip Kaludercic @ 2023-09-18 9:37 UTC (permalink / raw) To: Lynn Winebarger; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 245 bytes --] Lynn Winebarger <owinebar@gmail.com> writes: > I'd like to submit "tam" (table allocation manager) for inclusion in > GNU ELPA. The source is available at > https://github.com/owinebar/emacs-table-allocation-manager Here are a few comments: [-- Attachment #2: Type: text/plain, Size: 5048 bytes --] diff --git a/table-allocation-manager.el b/table-allocation-manager.el index 59a5718..286c9a2 100644 --- a/table-allocation-manager.el +++ b/table-allocation-manager.el @@ -3,6 +3,10 @@ ;; Copyright (C) 2023 Onnie Lynn Winebarger ;; Author: Onnie Lynn Winebarger <owinebar@gmail.com> +;; Maintainer: Onnie Lynn Winebarger <owinebar@gmail.com> +;; URL: https://github.com/owinebar/emacs-table-allocation-manager +;; Package-Requires: ((emacs "24.4") (queue "0.2")) + ;; Keywords: lisp, tools ;; This program is free software; you can redistribute it and/or modify @@ -24,7 +28,9 @@ ;; table. All allocation is done during initialization to avoid triggering ;; garbage collection during allocation/free operations. -;; API: +;; API: (is it necessary to document the API here? This has to be +;; kept up to date, while a M-x apropos-function tam- RET could avoid +;; the issue.) ;; ;; (tam-create-table N) => table of size N ;; (tam-table-fullp TABLE) => nil unless TABLE is full @@ -43,13 +49,12 @@ ;; (tam-table-free-list TABLE) => list of free indices in TABLE ;; (tam-table-live-list TABLE) => list of in-use indices in TABLE - ;;; Code: (eval-when-compile (require 'cl-lib)) -(require 'queue) +(require 'queue) ;is this even necessary? see below. (cl-defstruct (tam--table (:constructor tam--table-create (size)) (:copier tam--copy-table)) @@ -66,16 +71,15 @@ (table index in-use next previous)) (:copier tam--copy-slot)) "Slot in TAM table" - table ;; table containing this slot - index ;; index of slot in table - in-use ;; flag indicating if contents are "live" - next ;; next on list of used/free - previous ;; previous on list of used/free - contents ;; contents of slot - ) + (table :documentation "table containing this slot") + (index :documentation "index of slot in table") + (in-use :documentation "flag indicating if contents are live") + (next :documentation "next on list of used/free") + (previous :documentation "previous on list of used/free") + (contents :documentation "contents of slot")) (defun tam-create-table (N) - "Make a tam table of size N." + "Make a tam table of size N." ;"tam table" might not be clear. (let ((tbl (tam--table-create N)) (v (make-vector N nil)) (N-1 (- N 1)) @@ -98,8 +102,6 @@ (setf (tam--table-last-free tbl) (aref v N-1)) tbl)) - - (defun tam-table-fullp (tbl) "Test if TBL is full." (<= (tam--table-size tbl) (tam--table-used tbl))) @@ -108,22 +110,20 @@ "Test if TBL is empty." (= (tam--table-used tbl) 0)) -(defsubst tam-table-size (tbl) +(defsubst tam-table-size (tbl) ;why not `defalias' "Number of slots in TBL." (tam--table-size tbl)) - (defsubst tam-table-used (tbl) "Number of slots of TBL in use." (tam--table-used tbl)) (defun tam--table-get-slot (tbl idx) - "Get slot IDX of TBL" + "Get slot IDX of TBL." (aref (tam--table-slots tbl) idx)) - (defun tam-table-get (tbl idx) - "Get contents of slot IDX of TBL" + "Get contents of slot IDX of TBL." (tam--slot-contents (aref (tam--table-slots tbl) idx))) (defun tam-allocate (tbl obj) @@ -133,9 +133,14 @@ Returns index or nil if table is full." next idx) (when (not (tam-table-fullp tbl)) (setf (tam--slot-previous s) (tam--table-last-used tbl)) - (if (tam-table-emptyp tbl) - (setf (tam--table-first-used tbl) s) - (setf (tam--slot-next (tam--table-last-used tbl)) s)) + (setf (if (tam-table-emptyp tbl) + (tam--table-first-used tbl) + (tam--slot-next (tam--table-last-used tbl))) + s) + (setf (if (tam-table-emptyp tbl) + (tam--table-first-used tbl) + (tam--slot-next (tam--table-last-used tbl))) + s) (setf (tam--table-last-used tbl) s) (setq next (tam--slot-next s)) (setf (tam--table-first-free tbl) next) @@ -151,8 +156,9 @@ Returns index or nil if table is full." idx)) (defun tam-free (tbl idx) - "Free slot at IDX in TBL. Returns contents of slot IDX. -Signals an error if IDX is not in use." + "Free slot at IDX in TBL. +Returns contents of slot IDX. Signals an error if IDX is not in +use." (let ((s (tam--table-get-slot tbl idx)) (last-free (tam--table-last-free tbl)) prev next obj) @@ -185,17 +191,19 @@ Signals an error if IDX is not in use." (cl-decf (tam--table-used tbl)) obj)) +;; you appear to only use the queue to track a list of objects, right? +;; Why not this then: (defun tam-table-free-list (tbl) - "Return list of free indices in TBL" + "Return list of free indices in TBL." (let ((s (tam--table-first-free tbl)) - (q (queue-create))) + (q '())) (while s - (queue-enqueue q (tam--slot-index s)) + (push (tam--slot-index s) q) (setq s (tam--slot-next s))) - (queue-all q))) + (nreverse q))) ;iff even necessary (defun tam-table-live-list (tbl) - "Return list of live indices in TBL" + "Return list of live indices in TBL." (let ((s (tam--table-first-used tbl)) (q (queue-create))) (while s [-- Attachment #3: Type: text/plain, Size: 23 bytes --] -- Philip Kaludercic ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-18 9:37 ` Philip Kaludercic @ 2023-09-18 16:22 ` Lynn Winebarger 2023-09-18 17:02 ` Philip Kaludercic 0 siblings, 1 reply; 20+ messages in thread From: Lynn Winebarger @ 2023-09-18 16:22 UTC (permalink / raw) To: Philip Kaludercic; +Cc: emacs-devel On Mon, Sep 18, 2023 at 5:37 AM Philip Kaludercic <philipk@posteo.net> wrote: > > Lynn Winebarger <owinebar@gmail.com> writes: > > > I'd like to submit "tam" (table allocation manager) for inclusion in > > GNU ELPA. The source is available at > > https://github.com/owinebar/emacs-table-allocation-manager > > Here are a few comments: Thanks for taking a look. > > diff --git a/table-allocation-manager.el b/table-allocation-manager.el > index 59a5718..286c9a2 100644 > --- a/table-allocation-manager.el > +++ b/table-allocation-manager.el > @@ -3,6 +3,10 @@ > ;; Copyright (C) 2023 Onnie Lynn Winebarger > > ;; Author: Onnie Lynn Winebarger <owinebar@gmail.com> > +;; Maintainer: Onnie Lynn Winebarger <owinebar@gmail.com> > +;; URL: https://github.com/owinebar/emacs-table-allocation-manager > +;; Package-Requires: ((emacs "24.4") (queue "0.2")) > + Apologies, I renamed the library to tam.el and failed to note the changes I made to the renamed file did not get committed and pushed. > ;; Keywords: lisp, tools > > ;; This program is free software; you can redistribute it and/or modify > @@ -24,7 +28,9 @@ > ;; table. All allocation is done during initialization to avoid triggering > ;; garbage collection during allocation/free operations. > > -;; API: > +;; API: (is it necessary to document the API here? This has to be > +;; kept up to date, while a M-x apropos-function tam- RET could avoid > +;; the issue.) I thought it would be helpful to summarize the functionality for review. > ;; > ;; (tam-create-table N) => table of size N > ;; (tam-table-fullp TABLE) => nil unless TABLE is full > @@ -43,13 +49,12 @@ > ;; (tam-table-free-list TABLE) => list of free indices in TABLE > ;; (tam-table-live-list TABLE) => list of in-use indices in TABLE > > - > ;;; Code: > > (eval-when-compile > (require 'cl-lib)) > > -(require 'queue) > +(require 'queue) ;is this even necessary? see below. If it's a big deal, then no. Since queue is a GNU package, I preferred to not repeat code. > > (cl-defstruct (tam--table (:constructor tam--table-create (size)) > (:copier tam--copy-table)) > @@ -66,16 +71,15 @@ > (table index in-use next previous)) > (:copier tam--copy-slot)) > "Slot in TAM table" > - table ;; table containing this slot > - index ;; index of slot in table > - in-use ;; flag indicating if contents are "live" > - next ;; next on list of used/free > - previous ;; previous on list of used/free > - contents ;; contents of slot > - ) > + (table :documentation "table containing this slot") > + (index :documentation "index of slot in table") > + (in-use :documentation "flag indicating if contents are live") > + (next :documentation "next on list of used/free") > + (previous :documentation "previous on list of used/free") > + (contents :documentation "contents of slot")) > > (defun tam-create-table (N) > - "Make a tam table of size N." > + "Make a tam table of size N." ;"tam table" might not be clear. > (let ((tbl (tam--table-create N)) > (v (make-vector N nil)) > (N-1 (- N 1)) > @@ -98,8 +102,6 @@ > (setf (tam--table-last-free tbl) (aref v N-1)) > tbl)) > > - > - > (defun tam-table-fullp (tbl) > "Test if TBL is full." > (<= (tam--table-size tbl) (tam--table-used tbl))) > @@ -108,22 +110,20 @@ > "Test if TBL is empty." > (= (tam--table-used tbl) 0)) > > -(defsubst tam-table-size (tbl) > +(defsubst tam-table-size (tbl) ;why not `defalias' I tried defalias first, but got a byte-compiler error about a void variable. Which I found confusing, since it should be looking for a function definition, not a variable. I'm using 28.3. Some of the following should have already been fixed from when I ran checkdoc. > "Number of slots in TBL." > (tam--table-size tbl)) > > - > (defsubst tam-table-used (tbl) > "Number of slots of TBL in use." > (tam--table-used tbl)) > > (defun tam--table-get-slot (tbl idx) > - "Get slot IDX of TBL" > + "Get slot IDX of TBL." > (aref (tam--table-slots tbl) idx)) > > - > (defun tam-table-get (tbl idx) > - "Get contents of slot IDX of TBL" > + "Get contents of slot IDX of TBL." > (tam--slot-contents (aref (tam--table-slots tbl) idx))) > > (defun tam-allocate (tbl obj) > @@ -133,9 +133,14 @@ Returns index or nil if table is full." > next idx) > (when (not (tam-table-fullp tbl)) > (setf (tam--slot-previous s) (tam--table-last-used tbl)) > - (if (tam-table-emptyp tbl) > - (setf (tam--table-first-used tbl) s) > - (setf (tam--slot-next (tam--table-last-used tbl)) s)) > + (setf (if (tam-table-emptyp tbl) > + (tam--table-first-used tbl) > + (tam--slot-next (tam--table-last-used tbl))) > + s) Is this a personal stylistic preference, or a requirement? I'll change it if required, but I find computing the place inside a set form to be disconcerting if it's not required. For example, I wouldn't use a set form like (set (if P 'A 'B) some-value) in place of (if P (setq A some-value) (setq B some-value)) where I might be amenable to (set (opaque-function-call args ...) some-value) > + (setf (if (tam-table-emptyp tbl) > + (tam--table-first-used tbl) > + (tam--slot-next (tam--table-last-used tbl))) > + s) I'm assuming this is a typo. > (setf (tam--table-last-used tbl) s) > (setq next (tam--slot-next s)) > (setf (tam--table-first-free tbl) next) > @@ -151,8 +156,9 @@ Returns index or nil if table is full." > idx)) > > (defun tam-free (tbl idx) > - "Free slot at IDX in TBL. Returns contents of slot IDX. > -Signals an error if IDX is not in use." > + "Free slot at IDX in TBL. > +Returns contents of slot IDX. Signals an error if IDX is not in > +use." > (let ((s (tam--table-get-slot tbl idx)) > (last-free (tam--table-last-free tbl)) > prev next obj) > @@ -185,17 +191,19 @@ Signals an error if IDX is not in use." > (cl-decf (tam--table-used tbl)) > obj)) > > +;; you appear to only use the queue to track a list of objects, right? > +;; Why not this then: > (defun tam-table-free-list (tbl) > - "Return list of free indices in TBL" > + "Return list of free indices in TBL." > (let ((s (tam--table-first-free tbl)) > - (q (queue-create))) > + (q '())) > (while s > - (queue-enqueue q (tam--slot-index s)) > + (push (tam--slot-index s) q) > (setq s (tam--slot-next s))) > - (queue-all q))) > + (nreverse q))) ;iff even necessary I do want to return lists reflecting the ordering of the slots. I am not a fan of constructing an ordered structure only to reverse it. I can rewrite this to append to the tail using let-bound head and tail variables, but it seems excessive to avoid a single allocation of a queue structure. That said, these functions are primarily intended for debugging. > > (defun tam-table-live-list (tbl) > - "Return list of live indices in TBL" > + "Return list of live indices in TBL." > (let ((s (tam--table-first-used tbl)) > (q (queue-create))) > (while s > > -- > Philip Kaludercic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-18 16:22 ` Lynn Winebarger @ 2023-09-18 17:02 ` Philip Kaludercic 2023-09-19 15:38 ` Lynn Winebarger 0 siblings, 1 reply; 20+ messages in thread From: Philip Kaludercic @ 2023-09-18 17:02 UTC (permalink / raw) To: Lynn Winebarger; +Cc: emacs-devel Lynn Winebarger <owinebar@gmail.com> writes: > On Mon, Sep 18, 2023 at 5:37 AM Philip Kaludercic <philipk@posteo.net> wrote: >> >> Lynn Winebarger <owinebar@gmail.com> writes: >> >> > I'd like to submit "tam" (table allocation manager) for inclusion in >> > GNU ELPA. The source is available at >> > https://github.com/owinebar/emacs-table-allocation-manager >> >> Here are a few comments: > Thanks for taking a look. > >> >> diff --git a/table-allocation-manager.el b/table-allocation-manager.el >> index 59a5718..286c9a2 100644 >> --- a/table-allocation-manager.el >> +++ b/table-allocation-manager.el >> @@ -3,6 +3,10 @@ >> ;; Copyright (C) 2023 Onnie Lynn Winebarger >> >> ;; Author: Onnie Lynn Winebarger <owinebar@gmail.com> >> +;; Maintainer: Onnie Lynn Winebarger <owinebar@gmail.com> >> +;; URL: https://github.com/owinebar/emacs-table-allocation-manager >> +;; Package-Requires: ((emacs "24.4") (queue "0.2")) >> + > > Apologies, I renamed the library to tam.el and failed to note the > changes I made to the renamed file did not get committed and pushed. So what does that mean? >> ;; Keywords: lisp, tools >> >> ;; This program is free software; you can redistribute it and/or modify >> @@ -24,7 +28,9 @@ >> ;; table. All allocation is done during initialization to avoid triggering >> ;; garbage collection during allocation/free operations. >> >> -;; API: >> +;; API: (is it necessary to document the API here? This has to be >> +;; kept up to date, while a M-x apropos-function tam- RET could avoid >> +;; the issue.) > > I thought it would be helpful to summarize the functionality for review. That is true, but I wouldn't bother with maintaining this in the long term. Encouraging the usage of apropos commands is good anyway. >> ;; >> ;; (tam-create-table N) => table of size N >> ;; (tam-table-fullp TABLE) => nil unless TABLE is full >> @@ -43,13 +49,12 @@ >> ;; (tam-table-free-list TABLE) => list of free indices in TABLE >> ;; (tam-table-live-list TABLE) => list of in-use indices in TABLE >> >> - >> ;;; Code: >> >> (eval-when-compile >> (require 'cl-lib)) >> >> -(require 'queue) >> +(require 'queue) ;is this even necessary? see below. > > If it's a big deal, then no. Since queue is a GNU package, I > preferred to not repeat code. I am the kind of person who thinks twice about installing a package when it has dependencies. But if you prefer to use a package available on ELPA, then that is of course OK as well. >> >> (cl-defstruct (tam--table (:constructor tam--table-create (size)) >> (:copier tam--copy-table)) >> @@ -66,16 +71,15 @@ >> (table index in-use next previous)) >> (:copier tam--copy-slot)) >> "Slot in TAM table" >> - table ;; table containing this slot >> - index ;; index of slot in table >> - in-use ;; flag indicating if contents are "live" >> - next ;; next on list of used/free >> - previous ;; previous on list of used/free >> - contents ;; contents of slot >> - ) >> + (table :documentation "table containing this slot") >> + (index :documentation "index of slot in table") >> + (in-use :documentation "flag indicating if contents are live") >> + (next :documentation "next on list of used/free") >> + (previous :documentation "previous on list of used/free") >> + (contents :documentation "contents of slot")) >> >> (defun tam-create-table (N) >> - "Make a tam table of size N." >> + "Make a tam table of size N." ;"tam table" might not be clear. >> (let ((tbl (tam--table-create N)) >> (v (make-vector N nil)) >> (N-1 (- N 1)) >> @@ -98,8 +102,6 @@ >> (setf (tam--table-last-free tbl) (aref v N-1)) >> tbl)) >> >> - >> - >> (defun tam-table-fullp (tbl) >> "Test if TBL is full." >> (<= (tam--table-size tbl) (tam--table-used tbl))) >> @@ -108,22 +110,20 @@ >> "Test if TBL is empty." >> (= (tam--table-used tbl) 0)) >> >> -(defsubst tam-table-size (tbl) >> +(defsubst tam-table-size (tbl) ;why not `defalias' > > I tried defalias first, but got a byte-compiler error about a void > variable. Which I found confusing, since it should be looking for a > function definition, not a variable. I'm using 28.3. > Some of the following should have already been fixed from when I ran checkdoc. What did you do? That sounds like something was misquoted: (defalias 'tam-table-size #'tam--table-size) ? >> "Number of slots in TBL." >> (tam--table-size tbl)) >> >> - >> (defsubst tam-table-used (tbl) >> "Number of slots of TBL in use." >> (tam--table-used tbl)) >> >> (defun tam--table-get-slot (tbl idx) >> - "Get slot IDX of TBL" >> + "Get slot IDX of TBL." >> (aref (tam--table-slots tbl) idx)) >> >> - >> (defun tam-table-get (tbl idx) >> - "Get contents of slot IDX of TBL" >> + "Get contents of slot IDX of TBL." >> (tam--slot-contents (aref (tam--table-slots tbl) idx))) >> >> (defun tam-allocate (tbl obj) >> @@ -133,9 +133,14 @@ Returns index or nil if table is full." >> next idx) >> (when (not (tam-table-fullp tbl)) >> (setf (tam--slot-previous s) (tam--table-last-used tbl)) >> - (if (tam-table-emptyp tbl) >> - (setf (tam--table-first-used tbl) s) >> - (setf (tam--slot-next (tam--table-last-used tbl)) s)) >> + (setf (if (tam-table-emptyp tbl) >> + (tam--table-first-used tbl) >> + (tam--slot-next (tam--table-last-used tbl))) >> + s) > > Is this a personal stylistic preference, or a requirement? Nothing I say is a requirement, I should have made that explicit. More of a "look what you could also do, in case you are interested". > I'll > change it if required, but I find computing the place inside a set > form to be disconcerting if it's not required. For example, I > wouldn't use a set form like > (set (if P 'A 'B) some-value) > in place of > (if P (setq A some-value) (setq B some-value)) In that case, is there a reason you are using setf? > where I might be amenable to > (set (opaque-function-call args ...) some-value) > >> + (setf (if (tam-table-emptyp tbl) >> + (tam--table-first-used tbl) >> + (tam--slot-next (tam--table-last-used tbl))) >> + s) > I'm assuming this is a typo. Probably. >> (setf (tam--table-last-used tbl) s) >> (setq next (tam--slot-next s)) >> (setf (tam--table-first-free tbl) next) >> @@ -151,8 +156,9 @@ Returns index or nil if table is full." >> idx)) >> >> (defun tam-free (tbl idx) >> - "Free slot at IDX in TBL. Returns contents of slot IDX. >> -Signals an error if IDX is not in use." >> + "Free slot at IDX in TBL. >> +Returns contents of slot IDX. Signals an error if IDX is not in >> +use." >> (let ((s (tam--table-get-slot tbl idx)) >> (last-free (tam--table-last-free tbl)) >> prev next obj) >> @@ -185,17 +191,19 @@ Signals an error if IDX is not in use." >> (cl-decf (tam--table-used tbl)) >> obj)) >> >> +;; you appear to only use the queue to track a list of objects, right? >> +;; Why not this then: >> (defun tam-table-free-list (tbl) >> - "Return list of free indices in TBL" >> + "Return list of free indices in TBL." >> (let ((s (tam--table-first-free tbl)) >> - (q (queue-create))) >> + (q '())) >> (while s >> - (queue-enqueue q (tam--slot-index s)) >> + (push (tam--slot-index s) q) >> (setq s (tam--slot-next s))) >> - (queue-all q))) >> + (nreverse q))) ;iff even necessary > > I do want to return lists reflecting the ordering of the slots. I am > not a fan of constructing an ordered structure only to reverse it. FWIW this is standard practice, and what a cl-loop with collect would also expand to. And if I am not mistaken, this is more efficient, than accumulating a linked list in the right order to begin with (it is a difference of O(n) vs O(n^2), I believe). > I can rewrite this to append to the tail using let-bound head and tail > variables, but it seems excessive to avoid a single allocation of a > queue structure. > That said, these functions are primarily intended for debugging. Wouldn't that kind of a convenience be an argument against adding an extra dependency? >> >> (defun tam-table-live-list (tbl) >> - "Return list of live indices in TBL" >> + "Return list of live indices in TBL." >> (let ((s (tam--table-first-used tbl)) >> (q (queue-create))) >> (while s >> >> -- >> Philip Kaludercic -- Philip Kaludercic ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-18 17:02 ` Philip Kaludercic @ 2023-09-19 15:38 ` Lynn Winebarger 2023-09-20 8:26 ` Philip Kaludercic 0 siblings, 1 reply; 20+ messages in thread From: Lynn Winebarger @ 2023-09-19 15:38 UTC (permalink / raw) To: Philip Kaludercic; +Cc: emacs-devel On Mon, Sep 18, 2023 at 1:02 PM Philip Kaludercic <philipk@posteo.net> wrote: > > Apologies, I renamed the library to tam.el and failed to note the > > changes I made to the renamed file did not get committed and pushed. > > So what does that mean? That I don't use git enough to avoid rookie errors? I had already addressed some of the items you brought up in my working copy, they just hadn't been committed and pushed to the server. I think I've addressed all the items you brought up in the last version I pushed. > I am the kind of person who thinks twice about installing a package when > it has dependencies. But if you prefer to use a package available on > ELPA, then that is of course OK as well. I revised it to make use of cl-loop per your observation in the previous email. I'm not really a fan of the loop imperative DSL, but this case was simple enough (as you pointed out). Some packages (like queue) provide pretty basic data structures, so I think avoiding packages that depend on them only encourages reimplementing the wheel. However, I might be the world record holder for the number of elisp libraries loaded and included in a dump file. > >> -(defsubst tam-table-size (tbl) > >> +(defsubst tam-table-size (tbl) ;why not `defalias' > > > > I tried defalias first, but got a byte-compiler error about a void > > variable. Which I found confusing, since it should be looking for a > > function definition, not a variable. I'm using 28.3. > > Some of the following should have already been fixed from when I ran checkdoc. > > What did you do? That sounds like something was misquoted: > > (defalias 'tam-table-size #'tam--table-size) I forgot defalias is a function and not a macro. Thanks. > > I'll > > change it if required, but I find computing the place inside a set > > form to be disconcerting if it's not required. For example, I > > wouldn't use a set form like > > (set (if P 'A 'B) some-value) > > in place of > > (if P (setq A some-value) (setq B some-value)) > > In that case, is there a reason you are using setf? I'm confused - the documentation does not indicate that an "if" form would be a "place" that setf knows how to handle. The cl-lib doc does say it extends setf to handle macro expansions, but 'if' is a special form that does not macro-expand. I use setf because that's the cleanest way to set fields of a structure. It's more like setq than set (it's a macro for one thing) as far as I can tell. > > I do want to return lists reflecting the ordering of the slots. I am > > not a fan of constructing an ordered structure only to reverse it. > > FWIW this is standard practice, and what a cl-loop with collect would > also expand to. And if I am not mistaken, this is more efficient, than > accumulating a linked list in the right order to begin with (it is a > difference of O(n) vs O(n^2), I believe). Wait, did you mean cl-loop with collect will do the nreverse? I thought you meant it would keep track of the tail of the list and update it. Accumulating a list in the right order is only O(n^2) if you only keep the head of the list. The queue structure (or what I would write with let-bound variables) holds a reference to the last cons cell of the list to use in adding an entry on the end. It's probably negligible for very short lists, but it's just bad form. > > That said, these functions are primarily intended for debugging. > > Wouldn't that kind of a convenience be an argument against adding an > extra dependency? Only if I was trying to get a library included in startup.el, or if the dependency was not in GNU ELPA. Otherwise, why are people writing packages? They are not all stand-alone user interfacing programs. Some are just basic data structures and algorithms that haven't been included in core emacs for whatever reason. Queue is one of those. Lynn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-19 15:38 ` Lynn Winebarger @ 2023-09-20 8:26 ` Philip Kaludercic 2023-09-20 16:14 ` Lynn Winebarger 0 siblings, 1 reply; 20+ messages in thread From: Philip Kaludercic @ 2023-09-20 8:26 UTC (permalink / raw) To: Lynn Winebarger; +Cc: emacs-devel Lynn Winebarger <owinebar@gmail.com> writes: > On Mon, Sep 18, 2023 at 1:02 PM Philip Kaludercic <philipk@posteo.net> wrote: >> > Apologies, I renamed the library to tam.el and failed to note the >> > changes I made to the renamed file did not get committed and pushed. >> >> So what does that mean? > > That I don't use git enough to avoid rookie errors? I had already > addressed some of the items you brought up in my working copy, they > just hadn't been committed and pushed to the server. > I think I've addressed all the items you brought up in the last > version I pushed. Ok. What I wanted to make sure was that you didn't change the repository URL or something like that. Re your last change in [0], why use records directly instead of having the code being generated via cl-defstruct? The commit messages doesn't really explain your reasoning to me. [0] https://github.com/owinebar/emacs-table-allocation-manager/commit/bc654b6d687c67c5ad45218d6f45f95b8f1e0478 >> I am the kind of person who thinks twice about installing a package when >> it has dependencies. But if you prefer to use a package available on >> ELPA, then that is of course OK as well. > > I revised it to make use of cl-loop per your observation in the > previous email. I'm not really a fan of the loop imperative DSL, but > this case was simple enough (as you pointed out). > Some packages (like queue) provide pretty basic data structures, so I > think avoiding packages that depend on them only encourages > reimplementing the wheel. > However, I might be the world record holder for the number of elisp > libraries loaded and included in a dump file. >> >> -(defsubst tam-table-size (tbl) >> >> +(defsubst tam-table-size (tbl) ;why not `defalias' >> > >> > I tried defalias first, but got a byte-compiler error about a void >> > variable. Which I found confusing, since it should be looking for a >> > function definition, not a variable. I'm using 28.3. >> > Some of the following should have already been fixed from when I >> > ran checkdoc. >> >> What did you do? That sounds like something was misquoted: >> >> (defalias 'tam-table-size #'tam--table-size) > > I forgot defalias is a function and not a macro. Thanks. > >> > I'll >> > change it if required, but I find computing the place inside a set >> > form to be disconcerting if it's not required. For example, I >> > wouldn't use a set form like >> > (set (if P 'A 'B) some-value) >> > in place of >> > (if P (setq A some-value) (setq B some-value)) >> >> In that case, is there a reason you are using setf? > > I'm confused - the documentation does not indicate that an "if" form > would be a "place" that setf knows how to handle. The cl-lib doc does > say it extends setf to handle macro expansions, but 'if' is a special > form that does not macro-expand. > I use setf because that's the cleanest way to set fields of a > structure. It's more like setq than set (it's a macro for one thing) > as far as I can tell. The question was supposed to be more general, sorry for the confusion. I wanted to know if there was a reason you were using setf even when setq would be enough, but it really doesn't matter either way since setf on a symbol expands directly to setq. >> > I do want to return lists reflecting the ordering of the slots. I am >> > not a fan of constructing an ordered structure only to reverse it. >> >> FWIW this is standard practice, and what a cl-loop with collect would >> also expand to. And if I am not mistaken, this is more efficient, than >> accumulating a linked list in the right order to begin with (it is a >> difference of O(n) vs O(n^2), I believe). > > Wait, did you mean cl-loop with collect will do the nreverse? I > thought you meant it would keep track of the tail of the list and > update it. No, it uses nreverse: --8<---------------cut here---------------start------------->8--- (macroexpand-all '(cl-loop for i to 10 collect i)) (let* ((i 0) (--cl-var-- nil)) (while (<= i 10) (setq --cl-var-- (cons i --cl-var--)) (setq i (+ i 1))) (nreverse --cl-var--)) --8<---------------cut here---------------end--------------->8--- > Accumulating a list in the right order is only O(n^2) if you only keep > the head of the list. The queue structure (or what I would write with > let-bound variables) holds a reference to the last cons cell of the > list to use in adding an entry on the end. It's probably negligible > for very short lists, but it's just bad form. > >> > That said, these functions are primarily intended for debugging. >> >> Wouldn't that kind of a convenience be an argument against adding an >> extra dependency? > > Only if I was trying to get a library included in startup.el, or if > the dependency was not in GNU ELPA. Otherwise, why are people writing > packages? They are not all stand-alone user interfacing programs. > Some are just basic data structures and algorithms that haven't been > included in core emacs for whatever reason. Queue is one of those. These kinds of arguments lead to leftpad like situations, where people defer any slightly complicated functionality to a library. The last thing I want to see when installing a package is that it drags along dozens or even hundreds of recursive dependants, causing me to loose an overview of what I have installed and what is being installed. Every dependency a package brings with it (especially packages like dash & co.) is an argument against using it, imo. > Lynn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-20 8:26 ` Philip Kaludercic @ 2023-09-20 16:14 ` Lynn Winebarger 2023-09-20 17:30 ` Stefan Monnier via Emacs development discussions. 2023-09-21 16:38 ` Philip Kaludercic 0 siblings, 2 replies; 20+ messages in thread From: Lynn Winebarger @ 2023-09-20 16:14 UTC (permalink / raw) To: Philip Kaludercic; +Cc: emacs-devel On Wed, Sep 20, 2023 at 4:26 AM Philip Kaludercic <philipk@posteo.net> wrote: > Re your last change in [0], why use records directly instead of having > the code being generated via cl-defstruct? The commit messages doesn't > really explain your reasoning to me. The library is supposed to provide alloc/free functions that run in O(1) time to the extent that is possible for emacs-lisp code, for use in process sentinels and similar situations. I took a look at the byte-code generated for those two functions when using the cl-defstruct definitions, and the accessors and setters were not inlined. Aside from the unknown complexity of invoking those functions, every call has a risk of overflowing the current stack and requiring an additional stack segment be allocated. I rewrote the code so the only appearances of the call operator in the byte-code of those functions is for error signaling. I also provide an inlining version of each operation so library clients can avoid call instructions in their code. > >> I am the kind of person who thinks twice about installing a package when > >> it has dependencies. But if you prefer to use a package available on > >> ELPA, then that is of course OK as well. BTW, there's something ironic about this, since you actually appear to review most packages on GNU/NonGNU ELPA - how many users would be more familiar with the packages that might be installed from those archives? At any rate, it does not depend on any packages, or even cl-lib, now - though I have to revise the header to say so. > > The question was supposed to be more general, sorry for the confusion. > I wanted to know if there was a reason you were using setf even when > setq would be enough, but it really doesn't matter either way since setf > on a symbol expands directly to setq. If I had setf on a symbol, it was a typo. They are all gone now, since they did not get optimized out. > No, it uses nreverse: > > --8<---------------cut here---------------start------------->8--- > (macroexpand-all '(cl-loop for i to 10 collect i)) > (let* ((i 0) (--cl-var-- nil)) > (while (<= i 10) > (setq --cl-var-- (cons i --cl-var--)) > (setq i (+ i 1))) > (nreverse --cl-var--)) > --8<---------------cut here---------------end--------------->8--- Blech, I replaced it with a simple function. > These kinds of arguments lead to leftpad like situations, where people > defer any slightly complicated functionality to a library. The last > thing I want to see when installing a package is that it drags along > dozens or even hundreds of recursive dependants, causing me to loose an > overview of what I have installed and what is being installed. Every > dependency a package brings with it (especially packages like dash & > co.) is an argument against using it, imo. I don't think the leftpad situation (which I had to look up) can happen on GNU ELPA. Even if, despite the FSF's precautions, something had to be removed, I'm sure there would be plenty of warning. Lynn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-20 16:14 ` Lynn Winebarger @ 2023-09-20 17:30 ` Stefan Monnier via Emacs development discussions. 2023-09-21 4:21 ` Lynn Winebarger 2023-09-21 16:38 ` Philip Kaludercic 1 sibling, 1 reply; 20+ messages in thread From: Stefan Monnier via Emacs development discussions. @ 2023-09-20 17:30 UTC (permalink / raw) To: emacs-devel >> Re your last change in [0], why use records directly instead of having >> the code being generated via cl-defstruct? The commit messages doesn't >> really explain your reasoning to me. > The library is supposed to provide alloc/free functions that run in > O(1) time to the extent that is possible for emacs-lisp code, for use > in process sentinels and similar situations. I took a look at the > byte-code generated for those two functions when using the > cl-defstruct definitions, and the accessors and setters were > not inlined. That's odd. Could you give more details about what&how you checked that? > Aside from the unknown complexity of invoking those functions, every > call has a risk of overflowing the current stack and requiring an > additional stack segment be allocated. That would still be O(1) in my book. > Accumulating a list in the right order is only O(n^2) if you only keep > the head of the list. The queue structure (or what I would write with > let-bound variables) holds a reference to the last cons cell of the > list to use in adding an entry on the end. It's probably negligible > for very short lists, but it's just bad form. Using `push`es followed by `nreverse` is usually more efficient than either of those. It's a standard idiom in Lisp for good reasons. If you care about the constant of your O(1) above, I recommend you use `nreverse` here as well :-) Stefan ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-20 17:30 ` Stefan Monnier via Emacs development discussions. @ 2023-09-21 4:21 ` Lynn Winebarger 2023-09-21 13:59 ` Stefan Monnier 0 siblings, 1 reply; 20+ messages in thread From: Lynn Winebarger @ 2023-09-21 4:21 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 3453 bytes --] On Wed, Sep 20, 2023 at 1:32 PM Stefan Monnier via Emacs development discussions. <emacs-devel@gnu.org> wrote: > >> Re your last change in [0], why use records directly instead of having > >> the code being generated via cl-defstruct? The commit messages doesn't > >> really explain your reasoning to me. > > The library is supposed to provide alloc/free functions that run in > > O(1) time to the extent that is possible for emacs-lisp code, for use > > in process sentinels and similar situations. I took a look at the > > byte-code generated for those two functions when using the > > cl-defstruct definitions, and the accessors and setters were > > not inlined. > > That's odd. Could you give more details about what&how you checked that? For your reference, I reset the main branch of the github repo to the last cl-defstruct version and put the subsequent commits on branch 'feature/no-cl-lib'. I've attached the output of M-x disassemble for tam-free after byte-compiling each version. Excluding the tam-already-free error condition, the feature/no-cl-lib version contains only stack operations, constants, aref, aset, branches, and a single return. On an admittedly cursory inspection of bytecode.c, none of those operations do any consing, and there will be no calls to maybe_gc because the unconditional jumps are all forward. You're correct, the accessors and setfs _are_ inlined as arefs and asets. I mistook the initial call to tam--table-get-slot to be an accessor. There are a lot of type-of, all of which are excessive for this type of low-level function. tam--slot structs should only appear in tam--table and tam--pool structs and the slots vectors of those should contain only tam--slot structs. There are no public functions that return a slot or set an entry in a slots vector of a tam--table or tam--pool. In any case, every call operation invokes maybe_gc unconditionally, so there is a chance of a GC occuring during the execution of the byte-vector. > > > Aside from the unknown complexity of invoking those functions, every > > call has a risk of overflowing the current stack and requiring an > > additional stack segment be allocated. > > That would still be O(1) in my book. Is that actually guaranteed? I haven't looked to see if obtaining an additional stack segment might cause a gc or just a system call to get more memory. > > Accumulating a list in the right order is only O(n^2) if you only keep > > the head of the list. The queue structure (or what I would write with > > let-bound variables) holds a reference to the last cons cell of the > > list to use in adding an entry on the end. It's probably negligible > > for very short lists, but it's just bad form. > > Using `push`es followed by `nreverse` is usually more efficient than > either of those. It's a standard idiom in Lisp for good reasons. > If you care about the constant of your O(1) above, I recommend you use > `nreverse` here as well :-) I am now recalling this discussion. I suppose it does just transpose the setcdr operations into a separate (but much faster) loop implemented in C with much lower loop overhead. I only care about the allocate, free, claim, and release functions providing O(1) performance. A list is getting consed in the "free-list" and "live-list" reporting functions, after all. Thanks for taking the time to look it over. Lynn [-- Attachment #2: disassembled-cl-defstruct-tam-free.txt --] [-- Type: text/plain, Size: 6241 bytes --] byte code for tam-free: doc: Free slot at IDX in TBL. ... args: (arg1 arg2) 0 constant tam--table-get-slot 1 stack-ref 2 2 stack-ref 2 3 call 2 4 constant type-of 5 stack-ref 3 6 call 1 7 varref cl-struct-tam--table-tags 8 memq 9 goto-if-not-nil 1 12 constant signal 13 constant wrong-type-argument 14 constant tam--table 15 stack-ref 5 16 list2 17 call 2 18 discard 19:1 stack-ref 2 20 constant 5 21 aref 22 constant nil 23 dup 24 dup 25 constant type-of 26 stack-ref 5 27 call 1 28 varref cl-struct-tam--slot-tags 29 memq 30 goto-if-not-nil 2 33 constant signal 34 constant wrong-type-argument 35 constant tam--slot 36 stack-ref 7 38 list2 39 call 2 40 discard 41:2 stack-ref 4 42 constant 3 43 aref 44 goto-if-not-nil 3 47 constant signal 48 constant tam-already-free 49 constant format 50 constant "Attempt to free unused table entry %s" 51 stack-ref 9 53 call 2 54 call 2 55 discard 56:3 constant type-of 57 stack-ref 5 58 call 1 59 varref cl-struct-tam--slot-tags 60 memq 61 goto-if-not-nil 4 64 constant signal 65 constant wrong-type-argument 66 constant tam--slot 67 stack-ref 7 69 list2 70 call 2 71 discard 72:4 stack-ref 4 73 constant 5 74 aref 75 stack-set 3 77 constant type-of 78 stack-ref 5 79 call 1 80 varref cl-struct-tam--slot-tags 81 memq 82 goto-if-not-nil 5 85 constant signal 86 constant wrong-type-argument 87 constant tam--slot 88 stack-ref 7 90 list2 91 call 2 92 discard 93:5 stack-ref 4 94 constant 4 95 aref 96 stack-set 2 98 constant type-of 99 stack-ref 5 100 call 1 101 varref cl-struct-tam--slot-tags 102 memq 103 goto-if-not-nil 6 106 constant signal 107 constant wrong-type-argument 108 constant tam--slot 109 stack-ref 7 111 list2 112 call 2 113 discard 114:6 stack-ref 4 115 constant 6 116 aref 117 stack-set 1 119 constant type-of 120 stack-ref 5 121 call 1 122 varref cl-struct-tam--slot-tags 123 memq 124 goto-if-not-nil 7 127 constant signal 128 constant wrong-type-argument 129 constant tam--slot 130 stack-ref 7 132 list2 133 call 2 134 discard 135:7 stack-ref 4 136 constant 4 137 constant nil 138 aset 139 discard 140 stack-ref 2 141 goto-if-nil 9 144 constant type-of 145 stack-ref 3 146 call 1 147 varref cl-struct-tam--slot-tags 148 memq 149 goto-if-not-nil 8 152 constant signal 153 constant wrong-type-argument 154 constant tam--slot 155 stack-ref 5 156 list2 157 call 2 158 discard 159:8 stack-ref 2 160 constant 4 161 stack-ref 3 162 aset 163 discard 164 goto 11 167:9 constant type-of 168 stack-ref 7 170 call 1 171 varref cl-struct-tam--table-tags 172 memq 173 goto-if-not-nil 10 176 constant signal 177 constant wrong-type-argument 178 constant tam--table 179 stack-ref 9 181 list2 182 call 2 183 discard 184:10 stack-ref 6 186 constant 6 187 stack-ref 3 188 aset 189 discard 190:11 stack-ref 1 191 goto-if-nil 13 194 constant type-of 195 stack-ref 2 196 call 1 197 varref cl-struct-tam--slot-tags 198 memq 199 goto-if-not-nil 12 202 constant signal 203 constant wrong-type-argument 204 constant tam--slot 205 stack-ref 4 206 list2 207 call 2 208 discard 209:12 stack-ref 1 210 constant 5 211 stack-ref 4 212 aset 213 discard 214 goto 15 217:13 constant type-of 218 stack-ref 7 220 call 1 221 varref cl-struct-tam--table-tags 222 memq 223 goto-if-not-nil 14 226 constant signal 227 constant wrong-type-argument 228 constant tam--table 229 stack-ref 9 231 list2 232 call 2 233 discard 234:14 stack-ref 6 236 constant 7 237 stack-ref 4 238 aset 239 discard 240:15 stack-ref 3 241 goto-if-nil 18 244 constant type-of 245 stack-ref 4 246 call 1 247 varref cl-struct-tam--slot-tags 248 memq 249 goto-if-not-nil 16 252 constant signal 253 constant wrong-type-argument 254 constant tam--slot 255 stack-ref 6 257 list2 258 call 2 259 discard 260:16 stack-ref 3 261 constant 4 262 stack-ref 6 264 aset 265 discard 266 constant type-of 267 stack-ref 5 268 call 1 269 varref cl-struct-tam--slot-tags 270 memq 271 goto-if-not-nil 17 274 constant signal 275 constant wrong-type-argument 276 constant tam--slot 277 stack-ref 7 279 list2 280 call 2 281 discard 282:17 stack-ref 4 283 constant 5 284 stack-ref 5 285 aset 286 discard 287 goto 21 290:18 constant type-of 291 stack-ref 7 293 call 1 294 varref cl-struct-tam--table-tags 295 memq 296 goto-if-not-nil 19 299 constant signal 300 constant wrong-type-argument 301 constant tam--table 302 stack-ref 9 304 list2 305 call 2 306 discard 307:19 stack-ref 6 309 constant 4 310 stack-ref 6 312 aset 313 discard 314 constant type-of 315 stack-ref 5 316 call 1 317 varref cl-struct-tam--slot-tags 318 memq 319 goto-if-not-nil 20 322 constant signal 323 constant wrong-type-argument 324 constant tam--slot 325 stack-ref 7 327 list2 328 call 2 329 discard 330:20 stack-ref 4 331 constant 5 332 constant nil 333 aset 334 discard 335:21 constant type-of 336 stack-ref 7 338 call 1 339 varref cl-struct-tam--table-tags 340 memq 341 goto-if-not-nil 22 344 constant signal 345 constant wrong-type-argument 346 constant tam--table 347 stack-ref 9 349 list2 350 call 2 351 discard 352:22 stack-ref 6 354 constant 5 355 stack-ref 6 357 aset 358 discard 359 constant type-of 360 stack-ref 5 361 call 1 362 varref cl-struct-tam--slot-tags 363 memq 364 goto-if-not-nil 23 367 constant signal 368 constant wrong-type-argument 369 constant tam--slot 370 stack-ref 7 372 list2 373 call 2 374 discard 375:23 stack-ref 4 376 constant 3 377 constant nil 378 aset 379 discard 380 constant type-of 381 stack-ref 5 382 call 1 383 varref cl-struct-tam--slot-tags 384 memq 385 goto-if-not-nil 24 388 constant signal 389 constant wrong-type-argument 390 constant tam--slot 391 stack-ref 7 393 list2 394 call 2 395 discard 396:24 stack-ref 4 397 constant 6 398 constant nil 399 aset 400 discard 401 constant type-of 402 stack-ref 7 404 call 1 405 varref cl-struct-tam--table-tags 406 memq 407 goto-if-not-nil 25 410 constant signal 411 constant wrong-type-argument 412 constant tam--table 413 stack-ref 9 415 list2 416 call 2 417 discard 418:25 stack-ref 6 420 constant 2 421 stack-ref 8 423 constant 2 424 aref 425 sub1 426 aset 427 discard 428 return [-- Attachment #3: disassembled-no-cl-lib-tam-free.txt --] [-- Type: text/plain, Size: 2419 bytes --] byte code for tam-free: args: (arg1 arg2) 0 stack-ref 1 1 stack-ref 1 2 stack-ref 1 3 dup 4 stack-ref 2 5 stack-ref 1 6 dup 7 constant 3 8 aref 9 stack-set 1 11 stack-ref 1 12 aref 13 discardN-preserve-tos 2 15 stack-ref 1 16 dup 17 constant 5 18 aref 19 stack-set 1 21 stack-ref 1 22 dup 23 constant 2 24 aref 25 stack-set 1 27 constant nil 28 dup 29 stack-ref 4 30 dup 31 constant 3 32 aref 33 stack-set 1 35 goto-if-not-nil 1 38 constant signal 39 constant tam-already-free 40 constant format 41 constant "Attempt to free unused table entry %s" 42 stack-ref 6 44 call 2 45 call 2 46 discard 47:1 stack-ref 4 48 dup 49 constant 5 50 aref 51 stack-set 1 53 stack-set 2 55 stack-ref 4 56 dup 57 constant 4 58 aref 59 stack-set 1 61 stack-set 1 63 stack-ref 4 64 constant nil 65 stack-ref 1 66 constant 4 67 stack-ref 2 68 aset 69 discardN 3 71 stack-ref 1 72 goto-if-nil 2 75 stack-ref 1 76 stack-ref 1 77 stack-ref 1 78 constant 4 79 stack-ref 2 80 aset 81 discardN 3 83 goto 3 86:2 stack-ref 5 87 stack-ref 1 88 stack-ref 1 89 constant 6 90 stack-ref 2 91 aset 92 discardN 3 94:3 dup 95 goto-if-nil 4 98 dup 99 stack-ref 2 100 stack-ref 1 101 constant 5 102 stack-ref 2 103 aset 104 discardN 3 106 goto 5 109:4 stack-ref 5 110 stack-ref 2 111 stack-ref 1 112 constant 7 113 stack-ref 2 114 aset 115 discardN 3 117:5 stack-ref 3 118 goto-if-nil 6 121 stack-ref 3 122 stack-ref 5 123 stack-ref 1 124 constant 4 125 stack-ref 2 126 aset 127 discardN 3 129 stack-ref 4 130 stack-ref 4 131 stack-ref 1 132 constant 5 133 stack-ref 2 134 aset 135 discardN 3 137 goto 7 140:6 stack-ref 5 141 stack-ref 5 142 stack-ref 1 143 constant 4 144 stack-ref 2 145 aset 146 discardN 3 148 stack-ref 4 149 constant nil 150 stack-ref 1 151 constant 5 152 stack-ref 2 153 aset 154 discardN 3 156:7 stack-ref 5 157 stack-ref 5 158 stack-ref 1 159 constant 5 160 stack-ref 2 161 aset 162 discardN 3 164 stack-ref 4 165 constant nil 166 stack-ref 1 167 constant 3 168 stack-ref 2 169 aset 170 discardN 3 172 stack-ref 5 173 dup 174 dup 175 constant 2 176 aref 177 stack-set 1 179 sub1 180 stack-ref 1 181 constant 2 182 stack-ref 2 183 aset 184 discardN 7 186 stack-set 1 188 constant nil 189 stack-ref 1 190 dup 191 constant 6 192 aref 193 stack-set 1 195 stack-set 1 197 stack-ref 1 198 constant nil 199 stack-ref 1 200 constant 6 201 stack-ref 2 202 aset 203 discardN 3 205 return ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-21 4:21 ` Lynn Winebarger @ 2023-09-21 13:59 ` Stefan Monnier 2023-09-22 3:01 ` Lynn Winebarger 0 siblings, 1 reply; 20+ messages in thread From: Stefan Monnier @ 2023-09-21 13:59 UTC (permalink / raw) To: Lynn Winebarger; +Cc: emacs-devel > You're correct, the accessors and setfs _are_ inlined as arefs and > asets. Thanks for confirming. > accessor. There are a lot of type-of, all of which are excessive for > this type of low-level function. You might be able to remove those by playing with the optimization level, e.g.: (cl-declaim (optimize (speed 3))) > or tam--pool. In any case, every call operation invokes maybe_gc > unconditionally, so there is a chance of a GC occuring during the > execution of the byte-vector. The GC can occur right before or right after since the act of calling your functions will go through `maybe_gc`, in any case, so I doubt that will ever make a significant difference. >> > Aside from the unknown complexity of invoking those functions, every >> > call has a risk of overflowing the current stack and requiring an >> > additional stack segment be allocated. >> That would still be O(1) in my book. > Is that actually guaranteed? Yes. > I haven't looked to see if obtaining an additional stack segment might > cause a gc or just a system call to get more memory. AFAIK, Emacs is not run as a real-time task, so I wonder why you're worrying about that. [ And no, an additional stack segment should not trigger GC, AFAIK. ] Stefan ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-21 13:59 ` Stefan Monnier @ 2023-09-22 3:01 ` Lynn Winebarger 2023-09-22 3:23 ` Stefan Monnier 0 siblings, 1 reply; 20+ messages in thread From: Lynn Winebarger @ 2023-09-22 3:01 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On Thu, Sep 21, 2023 at 9:59 AM Stefan Monnier <monnier@iro.umontreal.ca> wrote: > > or tam--pool. In any case, every call operation invokes maybe_gc > > unconditionally, so there is a chance of a GC occuring during the > > execution of the byte-vector. > > The GC can occur right before or right after since the act of calling > your functions will go through `maybe_gc`, in any case, so I doubt > that will ever make a significant difference. This is true. Every path to executing a callback will call maybe_quit and maybe_gc. > > I haven't looked to see if obtaining an additional stack segment might > > cause a gc or just a system call to get more memory. > > AFAIK, Emacs is not run as a real-time task, so I wonder why you're > worrying about that. > [ And no, an additional stack segment should not trigger GC, AFAIK. ] I want to add minimal overhead so developers will feel free to use it or other infrastructure packages that make use of it. I'm also just curious. Exercises of this sort are a satisfying excuse to dig into emacs-lisp internals. Lynn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-22 3:01 ` Lynn Winebarger @ 2023-09-22 3:23 ` Stefan Monnier 0 siblings, 0 replies; 20+ messages in thread From: Stefan Monnier @ 2023-09-22 3:23 UTC (permalink / raw) To: Lynn Winebarger; +Cc: emacs-devel > I'm also just curious. Exercises of this sort are a satisfying excuse > to dig into emacs-lisp internals. Can't argue with that! Stefan ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-20 16:14 ` Lynn Winebarger 2023-09-20 17:30 ` Stefan Monnier via Emacs development discussions. @ 2023-09-21 16:38 ` Philip Kaludercic 1 sibling, 0 replies; 20+ messages in thread From: Philip Kaludercic @ 2023-09-21 16:38 UTC (permalink / raw) To: Lynn Winebarger; +Cc: emacs-devel Lynn Winebarger <owinebar@gmail.com> writes: > On Wed, Sep 20, 2023 at 4:26 AM Philip Kaludercic <philipk@posteo.net> wrote: >> Re your last change in [0], why use records directly instead of having >> the code being generated via cl-defstruct? The commit messages doesn't >> really explain your reasoning to me. > > The library is supposed to provide alloc/free functions that run in > O(1) time to the extent that is possible for emacs-lisp code, for use > in process sentinels and similar situations. I took a look at the > byte-code generated for those two functions when using the > cl-defstruct definitions, and the accessors and setters were not > inlined. Aside from the unknown complexity of invoking those > functions, every call has a risk of overflowing the current stack and > requiring an additional stack segment be allocated. > > I rewrote the code so the only appearances of the call operator in the > byte-code of those functions is for error signaling. I also provide > an inlining version of each operation so library clients can avoid > call instructions in their code. From what I understand this situation was resolved in the other subthread, right? >> >> I am the kind of person who thinks twice about installing a package when >> >> it has dependencies. But if you prefer to use a package available on >> >> ELPA, then that is of course OK as well. > > BTW, there's something ironic about this, since you actually appear to > review most packages on GNU/NonGNU ELPA - how many users would be more > familiar with the packages that might be installed from those > archives? > At any rate, it does not depend on any packages, or even cl-lib, now - > though I have to revise the header to say so. I don't know if this is ironic or not, I just don't like having too many packages installed in general? As I said, this is my personal taste and I am not forcing this onto anyone. >> >> The question was supposed to be more general, sorry for the confusion. >> I wanted to know if there was a reason you were using setf even when >> setq would be enough, but it really doesn't matter either way since setf >> on a symbol expands directly to setq. > > If I had setf on a symbol, it was a typo. They are all gone now, > since they did not get optimized out. OK. >> No, it uses nreverse: >> >> --8<---------------cut here---------------start------------->8--- >> (macroexpand-all '(cl-loop for i to 10 collect i)) >> (let* ((i 0) (--cl-var-- nil)) >> (while (<= i 10) >> (setq --cl-var-- (cons i --cl-var--)) >> (setq i (+ i 1))) >> (nreverse --cl-var--)) >> --8<---------------cut here---------------end--------------->8--- > Blech, I replaced it with a simple function. Another idea could be using mapcan, but usually requires calling a lambda expression -- at which point we have firmly reached the territory of micro-optimisation, and it makes no sense to progress any further without some substantial, empirical measurements and good technical explanations behind the observations. >> These kinds of arguments lead to leftpad like situations, where people >> defer any slightly complicated functionality to a library. The last >> thing I want to see when installing a package is that it drags along >> dozens or even hundreds of recursive dependants, causing me to loose an >> overview of what I have installed and what is being installed. Every >> dependency a package brings with it (especially packages like dash & >> co.) is an argument against using it, imo. > > I don't think the leftpad situation (which I had to look up) can > happen on GNU ELPA. Even if, despite the FSF's precautions, something > had to be removed, I'm sure there would be plenty of warning. The point isn't so much that something gets removed (as you say, ELPA plays the middle-man here by mirroring the repository contents), but rather that a downstream bug or even a malicious change in a deep dependency can cause much more harm, even if the inherent utility, ie. need for the package in the first place was not that significant in the first place. > Lynn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-18 2:28 [GNU ELPA] New package: tam Lynn Winebarger 2023-09-18 9:37 ` Philip Kaludercic @ 2023-09-18 19:26 ` Adam Porter 2023-09-19 15:48 ` Lynn Winebarger 2023-09-20 16:06 ` Stefan Monnier 2 siblings, 1 reply; 20+ messages in thread From: Adam Porter @ 2023-09-18 19:26 UTC (permalink / raw) To: owinebar; +Cc: emacs-devel Hi Lynn, This looks like a very interesting package. My only feedback is trivial: that the names of the functions could omit "table" from their names, because it would make them more concise; and since they all operate on tables, it doesn't seem necessary for their names to specify that. So, e.g., instead of: ;; (tam-create-table N) => table of size N ;; (tam-table-fullp TABLE) => nil unless TABLE is full ;; (tam-table-emptyp TABLE) => nil unless TABLE is empty ;; (tam-table-size TABLE) => number of slots in TABLE ;; (tam-table-used TABLE) => number of slots of TABLE in use ;; (tam-table-get TABLE IDX) => contents of TABLE slot at index IDX ;; (tam-allocate TABLE OBJ) => ;; if not full, assigns OBJ to contents of a free slot in TABLE, ;; and returns the index of the slot ;; if full, returns nil ;; (tam-free TABLE INDEX) => ;; if slot at INDEX of TABLE is in use, move to the free list and ;; return the object formerly held by the slot ;; if slot is already free, signal an error ;; (tam-table-free-list TABLE) => list of free indices in TABLE ;; (tam-table-live-list TABLE) => list of in-use indices in TABLE It could be: ;; (tam-create N) => table of size N ;; (tam-fullp TABLE) => nil unless TABLE is full ;; (tam-emptyp TABLE) => nil unless TABLE is empty ;; (tam-size TABLE) => number of slots in TABLE ;; (tam-used TABLE) => number of slots of TABLE in use ;; (tam-get TABLE IDX) => contents of TABLE slot at index IDX ;; (tam-allocate TABLE OBJ) => ;; if not full, assigns OBJ to contents of a free slot in TABLE, ;; and returns the index of the slot ;; if full, returns nil ;; (tam-free TABLE INDEX) => ;; if slot at INDEX of TABLE is in use, move to the free list and ;; return the object formerly held by the slot ;; if slot is already free, signal an error ;; (tam-free-list TABLE) => list of free indices in TABLE ;; (tam-live-list TABLE) => list of in-use indices in TABLE That 6-fewer-characters could sometimes make the difference between a form fitting on one line or two. :) Thanks for this contribution. Adam ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-18 19:26 ` Adam Porter @ 2023-09-19 15:48 ` Lynn Winebarger 2023-09-19 16:29 ` Adam Porter 2023-09-21 20:26 ` Richard Stallman 0 siblings, 2 replies; 20+ messages in thread From: Lynn Winebarger @ 2023-09-19 15:48 UTC (permalink / raw) To: Adam Porter; +Cc: emacs-devel On Mon, Sep 18, 2023 at 3:26 PM Adam Porter <adam@alphapapa.net> wrote: > > Hi Lynn, > > This looks like a very interesting package. My only feedback is > trivial: that the names of the functions could omit "table" from their > names, because it would make them more concise; and since they all > operate on tables, it doesn't seem necessary for their names to specify > that. > Thanks for taking a look. The reason is that I was thinking about adding functionality for pools of preallocated objects that leverage the tables for tracking which objects are actually being used. I went ahead and wrote some basic support for such pools in the latest version pushed. It's possible the two data structures (tables and pools) could be unified, but I want the tables to be able to track arbitrary data associated with some nominal limited resource - the slots are preallocated, but any other associated data is not. For the scheduler example, I may allow 24 simultaneous async processes to run, but I could construct hundreds or thousands of job specifications that are waiting to get scheduled. I might use the pool object to pre-allocate some largish number of those to avoid any allocation in a process sentinel. Of course, this introduces the possibility of use-after-free and using unallocated objects (they are represented by simple indexes), so this structure should only be used with appropriate caution. Lynn ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-19 15:48 ` Lynn Winebarger @ 2023-09-19 16:29 ` Adam Porter 2023-09-21 20:26 ` Richard Stallman 1 sibling, 0 replies; 20+ messages in thread From: Adam Porter @ 2023-09-19 16:29 UTC (permalink / raw) To: Lynn Winebarger; +Cc: emacs-devel Hi Lynn, On 9/19/23 10:48, Lynn Winebarger wrote: > Thanks for taking a look. The reason is that I was thinking about > adding functionality for pools of preallocated objects that leverage > the tables for tracking which objects are actually being used. I went > ahead and wrote some basic support for such pools in the latest > version pushed. Of course, I should have known that you're several steps ahead of me. Forgive the noise. :) Looking forward to trying the library soon! --Adam ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-19 15:48 ` Lynn Winebarger 2023-09-19 16:29 ` Adam Porter @ 2023-09-21 20:26 ` Richard Stallman 2023-09-22 19:45 ` Adam Porter 1 sibling, 1 reply; 20+ messages in thread From: Richard Stallman @ 2023-09-21 20:26 UTC (permalink / raw) To: Lynn Winebarger; +Cc: adam, emacs-devel [[[ To any NSA and FBI agents reading my email: please consider ]]] [[[ whether defending the US Constitution against all enemies, ]]] [[[ foreign or domestic, requires you to follow Snowden's example. ]]] > Thanks for taking a look. The reason is that I was thinking about > adding functionality for pools of preallocated objects that leverage > the tables for tracking which objects are actually being used. I don't have much of a picture of what you're using this for. For all I know, maybe it is a good method for that use. But it does seem un-lispy. -- Dr Richard Stallman (https://stallman.org) Chief GNUisance of the GNU Project (https://gnu.org) Founder, Free Software Foundation (https://fsf.org) Internet Hall-of-Famer (https://internethalloffame.org) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-21 20:26 ` Richard Stallman @ 2023-09-22 19:45 ` Adam Porter 0 siblings, 0 replies; 20+ messages in thread From: Adam Porter @ 2023-09-22 19:45 UTC (permalink / raw) To: rms, Lynn Winebarger; +Cc: emacs-devel On 9/21/23 15:26, Richard Stallman wrote: > > Thanks for taking a look. The reason is that I was thinking about > > adding functionality for pools of preallocated objects that leverage > > the tables for tracking which objects are actually being used. > > I don't have much of a picture of what you're using this for. > For all I know, maybe it is a good method for that use. > > But it does seem un-lispy. Agreed. I am interested in it, but I'd like to know more about what problem it's intended to solve. e.g. I've had functions that certainly do allocation and GC in a process sentinel, and AFAIK that hasn't been a problem. But a project I'm working on may exercise this kind of functionality more in the future, so maybe tam.el could be helpful. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-18 2:28 [GNU ELPA] New package: tam Lynn Winebarger 2023-09-18 9:37 ` Philip Kaludercic 2023-09-18 19:26 ` Adam Porter @ 2023-09-20 16:06 ` Stefan Monnier 2023-09-20 16:44 ` Lynn Winebarger 2 siblings, 1 reply; 20+ messages in thread From: Stefan Monnier @ 2023-09-20 16:06 UTC (permalink / raw) To: Lynn Winebarger; +Cc: emacs-devel Any reason why you're not using `cl-defstruct`? Also, is it important to document that the values returned from `tam-allocate` (and passed to `tam-free/tam-table-get`) are integers? Stefan ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [GNU ELPA] New package: tam 2023-09-20 16:06 ` Stefan Monnier @ 2023-09-20 16:44 ` Lynn Winebarger 0 siblings, 0 replies; 20+ messages in thread From: Lynn Winebarger @ 2023-09-20 16:44 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On Wed, Sep 20, 2023 at 12:07 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote: > Any reason why you're not using `cl-defstruct`? See my last response to Philip Kaludercic. > Also, is it important to document that the values returned from > `tam-allocate` (and passed to `tam-free/tam-table-get`) are integers? I'm only committed to those functions returning an "index" that can be created in constant time. Since there is no facility for multiple return values without some kind of consing, that pretty much means a fixnum. I have at least considered making the integer return value into a bitfield split between a slot number and some kind of "generation number" recorded in each slot, so free-after-free errors with an intervening reallocation could be caught. That would be over-engineering at this point, but I wouldn't commit to never providing it as an option. Maybe I should rename tam-table-get to just tam-get now that the table and pool structures overlap. Lynn ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-09-22 19:45 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-18 2:28 [GNU ELPA] New package: tam Lynn Winebarger 2023-09-18 9:37 ` Philip Kaludercic 2023-09-18 16:22 ` Lynn Winebarger 2023-09-18 17:02 ` Philip Kaludercic 2023-09-19 15:38 ` Lynn Winebarger 2023-09-20 8:26 ` Philip Kaludercic 2023-09-20 16:14 ` Lynn Winebarger 2023-09-20 17:30 ` Stefan Monnier via Emacs development discussions. 2023-09-21 4:21 ` Lynn Winebarger 2023-09-21 13:59 ` Stefan Monnier 2023-09-22 3:01 ` Lynn Winebarger 2023-09-22 3:23 ` Stefan Monnier 2023-09-21 16:38 ` Philip Kaludercic 2023-09-18 19:26 ` Adam Porter 2023-09-19 15:48 ` Lynn Winebarger 2023-09-19 16:29 ` Adam Porter 2023-09-21 20:26 ` Richard Stallman 2023-09-22 19:45 ` Adam Porter 2023-09-20 16:06 ` Stefan Monnier 2023-09-20 16:44 ` Lynn Winebarger
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.