Cleaning

[mgi]

Hi,

Here is a work to remove the old def-clx-class and bring common lisp xlib to post ANSI normalization defclass (like it's 1994!).

I've mostly tested this code with:

(ql:quickload :clim-examples)
(clim-demo:demodemo)

Ok all those patches could have been done with a simple (setf xlib::*def-clx-class-use-defclass t). But maybe, it is time to kill this machinery and stick to real CLOS class, no?

[dkochmanski]

I'm all for removing obsolete cruft, but this PR breaks CCL. Please make sure that (clim-demo:demodemo) starts fine on at least SBCL and CCL (I will additionally test on ECL and run other tests).

ccl debugger output (using CLX from your branch):

CL-USER> (ql:quickload 'clx)
CL-USER> (clim-demo:demodemo)

Undefined function XLIB::DISPLAY-OUTPUT-STREAM called with arguments (#<DISPLAY :0 ( R0)>) .
   [Condition of type CCL::UNDEFINED-FUNCTION-CALL]

Restarts:
 0: [CONTINUE] Retry applying XLIB::DISPLAY-OUTPUT-STREAM to (#<DISPLAY :0 ( R0)>).
 1: [USE-VALUE] Apply specified function to (#<DISPLAY :0 ( R0)>) this time.
 2: [STORE-VALUE] Specify a function to use as the definition of XLIB::DISPLAY-OUTPUT-STREAM.
 3: [RETRY] Retry SLIME REPL evaluation request.
 4: [*ABORT] Return to SLIME's top level.
 5: [ABORT-BREAK] Reset this thread
 --more--

Backtrace:
  0: (XLIB::BUFFER-WRITE-DEFAULT #(108 0 11 0 0 0 ...) #<DISPLAY :0 ( R0)> 0 12)
  1: (XLIB::BUFFER-WRITE #(108 0 11 0 0 0 ...) #<DISPLAY :0 ( R0)> 0 12)
  2: (XLIB::BUFFER-FLUSH #<DISPLAY :0 ( R0)>)
  3: (XLIB::BUFFER-FORCE-OUTPUT #<DISPLAY :0 ( R0)>)
  4: (XLIB::DISPLAY-CONNECT #<DISPLAY :0 ( R0)> :AUTHORIZATION-NAME "" :AUTHORIZATION-DATA "")
  5: (XLIB:OPEN-DISPLAY "" :DISPLAY 0 :PROTOCOL :LOCAL :AUTHORIZATION-NAME "" :AUTHORIZATION-DATA "")
  6: (#<STANDARD-METHOD CLIM-CLX::INITIALIZE-CLX (CLIM-CLX::CLX-BASIC-PORT)> #<CLX-PORT  #x302004DFCEAD>)
  7: (#<STANDARD-METHOD INITIALIZE-INSTANCE :AFTER (CLIM-CLX::CLX-PORT)> #<CLX-PORT  #x302004DFCEAD> :SERVER-PATH (:CLX :HOST "" :DISPLAY-ID 0 :SCREEN-ID ...))
  8: (CCL::%%BEFORE-AND-AFTER-COMBINED-METHOD-DCODE (NIL #<STANDARD-METHOD INITIALIZE-INSTANCE :AFTER (CLIM-CLX::CLX-PORT)> . 17587291331214))
  9: (CCL::%%STANDARD-COMBINED-METHOD-DCODE (NIL (#<STANDARD-METHOD INITIALIZE-INSTANCE :AFTER (CLIM:PORT)> #<STANDARD-METHOD INITIALIZE-INSTANCE :AFTER #>) ..) 17587291331214)

[dkochmanski]

also please don't use #5 in commits, because hash-number causes github to reference issues on the trackers (and it may be confusing at times, since commits are in fact not revelant to the tickets).

[mgi]

Daniel Kochmanski notifications@github.com writes:

I'm all for removing obsolete cruft, but this PR breaks CCL. Please make sure that (clim-demo:demodemo) starts fine on at least SBCL and CCL (I will additionally test on ECL and run other tests).

Hi Daniel,

I'm running openbsd and don't have something I could run CCL on handy. I've commited a patch earlier in this same branch that (hopefully) should fix this first error.

Maybe you could try it while I'll try to get a CCL up and running.

(noted for the hash notation in github :) Best regards, -- Manuel Giraud

[mgi]

Yes, i too think i'm lowering CCL but I can't make it work with class accessors. Example:

(defstruct foo a)
(defparameter *afoo* (make-foo :a 10))
(ccl::conditional-store (foo-a *afoo*) 10 1) ;; works as expected

(defclass bar () ((a :initarg :a :accessor bar-a)))
(defparameter *abar* (make-instance 'bar :a 10))
(ccl::conditional-store (bar-a *abar*) 10 1) ;; ends up in the debugger with:
                                             ;; "Don't know how to do conditional store to (BAR-A *ABAR*)"

And that is exactly what happen now that everything is a class...

[dkochmanski]

Since everything is a class now, could we make some benchmarks what is a performance hit? because if CLX works noticebly faster with old clx-class, then I'm not so sure if we want to make that switch..

[dkochmanski]

;;; From CLX.

;;; The good news is that this uses an interlocked load/store sequence
;;; and is fairly efficient.
;;; The bad news is that it only handles a few types of "place" forms.
;;; The good news is that CLX only uses a few types of "place" forms.

(defmacro conditional-store (place old-value new-value &environment env)
  (setq place (macroexpand place env))
  (if (atom place)
    ;; CLX uses special variables' value cells as place forms.
    (if (and (symbolp place)
             (eq :special (ccl::variable-information place env)))
      (let* ((base (gensym))
             (offset (gensym)))
        `(multiple-value-bind (,base ,offset)
          (ccl::%symbol-binding-address ',place)
          (ccl::%store-node-conditional ,offset ,base ,old-value ,new-value)))
      (signal-program-error "~s is not a special variable ." place))
    (let* ((sym (car place))
           (struct-transform (structref-info sym env)))
      (if struct-transform
        (setq place (defstruct-ref-transform struct-transform (cdr place) env)
              sym (car place)))
      (if (member  sym '(svref ccl::%svref ccl::struct-ref))
        (let* ((v (gensym)))
          `(let* ((,v ,(cadr place)))
            (ccl::store-gvector-conditional ,(caddr place)
             ,v ,old-value ,new-value)))
        (signal-program-error "Don't know how to do conditional store to ~s" place)))))

conditional-store from ccl, it looks that it should work fine with structures.

[mgi]

The whole point of this branch was to switch to standard defclass instead of having the def-clx-class layer. I think it is more a maintainably patch then a performance patch.

[dkochmanski]

Yes, that's what I understood from this PR. It just occured to me right now, that we may suffer a big performance penalty. Implementations are usually quite good when they optimize code using defstruct - not so much with CLOS classes.

So maybe it is worth considering replacing def-clx-class with

(defstruct (,name ,@options)
       ,@slots)

instead of defclass?

[mgi]

At this point, I think we need benchmarks (as you said). Because, I find defclass clearer, maybe more verbose, but just enough considering there is no auto-generation of functions, constructor,... And you get real inheritance for free.

But, I don't know how to write such a benchmark :-/

[mgi]

Hi,

While musing on the interweb, I found this post by Rainer Joswig (https://www.reddit.com/r/lisp/comments/70qsbk/going_faster_with_lisp/dn5ti5z/) comparing struct vs CLOS class.

Maybe we should stick to defstruct then in clx for performance reason. @PuercoPop: do you have any other arguments here in favor of CLOS class?

@dkochmanski : Tell me, if this the way you want to go and then I can do a new branch to substitute def-clx-class to defstruct (I still think it is good for maintainability)

[PuercoPop]

performance reasons

Keep in mind that XELB, written in Elisp, gets away with using EIOIO to represent events and replies and generic functions to serialiaze/deserialize them. I'd be amazed if EIOIO is faster than CLOS, even with their impoverished feature set.

Most likely, given that this is a network library, the process is I/O bound. I'd be surprised if this change would result in a performance hit at all.

One way to benchmark this would be to instrument a WM to log all events received along with a timestamp. Using that we can get a approximate idea of the 'event profile' and an idea of what the 'maximum event load' for a period of ~300ms. With that we can write a client program that tries to write as many events as possible in, say 10 seconds, trying to keep a realistic distribution of events.

Even if you decide to go with the defstruct route I'd be happy to see def-clx-class go away.

If y'all want, I could instrument a StumpWM to do record events this weekend and share the branch (but not the resulting log for privacy reasons).

[mgi]

Javier Olaechea notifications@github.com writes:

Most likely, given that this is a network library, the process is I/O bound. I'd be surprised if this change would result in a performance hit at all.

Yes, good one. The article I was referring to was about CPU emulation! I think performance far less important here. So I still think defclass is better move.

If y'all want, I could instrument a StumpWM to do record events this weekend and share the branch (but not the resulting log for privacy reasons).

Could you come up with an anonymized conclusion of your usage ;-) On my side, I'm using this branch with my wm for days without noticing any glitch. -- Manuel Giraud

[PuercoPop]

Update, although I haven't ran any benchmarks yet, I've been running this branch on my day to day w/o experiencing any issues.

[dkochmanski]

as mentioned, I'd like to see benchmarks showing, how clx is bound. if using defclass proves to be slower, then we should stick to defstruct (or keep both for people insisting they need all the CLOS cruft).

[dkochmanski]

bump ;)

[mgi]

Daniel Kochmanski notifications@github.com writes:

bump ;)

Yes sorry, I have to do this benchmarking but don't really know where to start. (as for the thumb up, it is a mistake I've already done in the github ux :-) -- Manuel Giraud

[dkochmanski]

clim-examples (clim-demo:demodemo) has a "Drawing Benchmark" test. You may compare results when ran with different structure implementations.

Keep in mind, that you have to remove cache (~/.cache/common-lisp/) to make sure that no fasl is taken from before. Moreover I want you to check it on SBCL, CCL and ECL (that gives 6 runs – 2 CLX times 3 implementations). If there is no significant difference (between different CLX on the same implementation) I'm fine with merging it (given it passes all tests, we've added some in the meantime).

I know that such benchmark is flawed in many ways, but this should be relatively easy to perform and goes through "warmed up" CLOS cache because of numerous repetitions.

[mgi]

I cannot make McCLIM to compile on ECL 16.1.3 (stuck with illegal instruction on babel compilation), so I come up with following benchmark code:

(ql:quickload :clx)
(ql:quickload :trivial-benchmark)

(defun bench/create-map-unmap-destroy-window ()
  (trivial-benchmark:with-timing (10000)
    (let* ((dpy (xlib:open-default-display))
       (root (xlib:screen-root (xlib:display-default-screen dpy)))
       (win (xlib:create-window :parent root :x 0 :y 0 :width 100 :height 100)))
      (xlib:map-window win)
      (xlib:unmap-window win)
      (xlib:destroy-window win)
      (xlib:close-display dpy))))

(defun bench/draw-rectangle-red ()
  (let* ((sz 100)
     (dpy (xlib:open-default-display))
     (root (xlib:screen-root (xlib:display-default-screen dpy)))
     (cmap (xlib:screen-default-colormap (xlib:display-default-screen dpy)))
     (win (xlib:create-window :parent root :x 0 :y 0 :width sz :height sz))
     (gc (xlib:create-gcontext :drawable win)))
    (xlib:map-window win)
    (setf (xlib:gcontext-foreground gc)
      (xlib:alloc-color cmap (xlib:make-color :red 1.0
                          :green 0.0
                          :blue 0.0)))
    (trivial-benchmark:with-timing (50000)
      (xlib:draw-rectangle win gc 0 0 sz sz t))
    (xlib:unmap-window win)
    (xlib:free-gcontext gc)
    (xlib:destroy-window win)
    (xlib:close-display dpy)))

(defun bench/draw-rectangle-random ()
  (let* ((sz 100)
     (dpy (xlib:open-default-display))
     (root (xlib:screen-root (xlib:display-default-screen dpy)))
     (cmap (xlib:screen-default-colormap (xlib:display-default-screen dpy)))
     (win (xlib:create-window :parent root :x 0 :y 0 :width sz :height sz))
     (gc (xlib:create-gcontext :drawable win)))
    (xlib:map-window win)
    (trivial-benchmark:with-timing (50000)
      (setf (xlib:gcontext-foreground gc)
        (xlib:alloc-color cmap (xlib:make-color :red (random 1.0)
                            :green (random 1.0)
                            :blue (random 1.0))))
      (xlib:draw-rectangle win gc 0 0 sz sz t))
    (xlib:unmap-window win)
    (xlib:free-gcontext gc)
    (xlib:destroy-window win)
    (xlib:close-display dpy)))

Every run was done from a clean ".cache/common-lisp". Note that CCL is on a different machine as it is not ported to OpenBSD.

SBCL

Results for clx-master on sbcl 1.4.7.98/OpenBSD/i5-6300U CPU @ 2.40GHz

CL-USER> (bench/create-map-unmap-destroy-window)
-                SAMPLES  TOTAL      MINIMUM  MAXIMUM  MEDIAN  AVERAGE    DEVIATION  
REAL-TIME        10000    5.782      0        0.099    0.001   0.000578   0.001127   
RUN-TIME         10000    4.01       0        0.1      0       0.000401   0.002179   
USER-RUN-TIME    10000    1.94       0        0.07     0       0.000194   0.001524   
SYSTEM-RUN-TIME  10000    2.01       0        0.03     0       0.000201   0.001425   
PAGE-FAULTS      10000    0          0        0        0       0          0.0        
GC-RUN-TIME      10000    0.18       0        0.1      0       0.000018   0.001039   
BYTES-CONSED     10000    645441824  31664    896720   64464   64544.184  8406.005   
EVAL-CALLS       10000    0          0        0        0       0          0.0        
NIL
CL-USER> (bench/draw-rectangle-red)
-                SAMPLES  TOTAL  MINIMUM  MAXIMUM  MEDIAN  AVERAGE   DEVIATION  
REAL-TIME        50000    0.348  0        0.001    0       0.000007  0.000083   
RUN-TIME         50000    0.34   0        0.01     0       0.000007  0.000261   
USER-RUN-TIME    50000    0.13   0        0.01     0       0.000003  0.000161   
SYSTEM-RUN-TIME  50000    0.19   0        0.01     0       0.000004  0.000195   
PAGE-FAULTS      50000    0      0        0        0       0         0.0        
GC-RUN-TIME      50000    0      0        0        0       0         0.0        
BYTES-CONSED     50000    0      0        0        0       0         0.0        
EVAL-CALLS       50000    0      0        0        0       0         0.0        
NIL
CL-USER> (bench/draw-rectangle-random)
-                SAMPLES  TOTAL    MINIMUM  MAXIMUM  MEDIAN  AVERAGE   DEVIATION  
REAL-TIME        50000    3.441    0        0.003    0       0.000069  0.000253   
RUN-TIME         50000    1.53     0        0.01     0       0.000031  0.000552   
USER-RUN-TIME    50000    0.64     0        0.01     0       0.000013  0.000358   
SYSTEM-RUN-TIME  50000    0.91     0        0.01     0       0.000018  0.000426   
PAGE-FAULTS      50000    0        0        0        0       0         0.0        
GC-RUN-TIME      50000    0        0        0        0       0         0.0        
BYTES-CONSED     50000    3309536  0        131056   0       66.19072  1556.3324  
EVAL-CALLS       50000    0        0        0        0       0         0.0        
NIL

Results for clx-cleaning on sbcl 1.4.7.98/OpenBSD/i5-6300U CPU @ 2.40GHz

CL-USER> (bench/create-map-unmap-destroy-window)
-                SAMPLES  TOTAL      MINIMUM  MAXIMUM   MEDIAN  AVERAGE   DEVIATION  
REAL-TIME        10000    6.362      0        0.134     0.001   0.000636  0.001507   
RUN-TIME         10000    4.26       0        0.1       0       0.000426  0.002249   
USER-RUN-TIME    10000    2.38       0        0.05      0       0.000238  0.001607   
SYSTEM-RUN-TIME  10000    1.9        0        0.05      0       0.00019   0.001437   
PAGE-FAULTS      10000    0          0        0         0       0         0.0        
GC-RUN-TIME      10000    0.3        0        0.1       0       0.00003   0.001104   
BYTES-CONSED     10000    674731824  54272    11296432  62896   67473.18  112936.94  
EVAL-CALLS       10000    0          0        0         0       0         0.0        
NIL
CL-USER> (bench/draw-rectangle-red)
-                SAMPLES  TOTAL  MINIMUM  MAXIMUM  MEDIAN  AVERAGE   DEVIATION  
REAL-TIME        50000    0.36   0        0.001    0       0.000007  0.000085   
RUN-TIME         50000    0.37   0        0.01     0       0.000007  0.000272   
USER-RUN-TIME    50000    0.1    0        0.01     0       0.000002  0.000141   
SYSTEM-RUN-TIME  50000    0.25   0        0.01     0       0.000005  0.000224   
PAGE-FAULTS      50000    0      0        0        0       0         0.0        
GC-RUN-TIME      50000    0      0        0        0       0         0.0        
BYTES-CONSED     50000    0      0        0        0       0         0.0        
EVAL-CALLS       50000    0      0        0        0       0         0.0        
NIL
CL-USER> (bench/draw-rectangle-random)
-                SAMPLES  TOTAL    MINIMUM  MAXIMUM  MEDIAN  AVERAGE    DEVIATION  
REAL-TIME        50000    3.53     0        0.001    0       0.000071   0.000256   
RUN-TIME         50000    1.63     0        0.01     0       0.000033   0.00057    
USER-RUN-TIME    50000    0.82     0        0.01     0       0.000016   0.000405   
SYSTEM-RUN-TIME  50000    0.81     0        0.01     0       0.000016   0.000402   
PAGE-FAULTS      50000    0        0        0        0       0          0.0        
GC-RUN-TIME      50000    0        0        0        0       0          0.0        
BYTES-CONSED     50000    6583744  0        131040   0       131.67488  2143.8723  
EVAL-CALLS       50000    0        0        0        0       0          0.0        
NIL

ECL

Results for clx-master on ecl 16.1.3/OpenBSD/i5-6300U CPU @ 2.40GHz

CL-USER> (bench/create-map-unmap-destroy-window)
-             SAMPLES  TOTAL      MINIMUM  MAXIMUM  MEDIAN  AVERAGE    DEVIATION  
REAL-TIME     10000    11.36      0        0.026    0.001   0.001136   0.000786   
RUN-TIME      10000    7.74       0        0.03     0       0.000774   0.002735   
BYTES-CONSED  10000    521064640  52096    106368   52096   52106.465  654.82904  
NIL
CL-USER> (bench/draw-rectangle-red)
-             SAMPLES  TOTAL    MINIMUM  MAXIMUM  MEDIAN  AVERAGE   DEVIATION  
REAL-TIME     50000    0.634    0        0.001    0       0.000013  0.000112   
RUN-TIME      50000    0.42     0        0.01     0       0.000008  0.00029    
BYTES-CONSED  50000    4000000  80       80       80      80        0.0        
NIL
CL-USER> (bench/draw-rectangle-random)
-             SAMPLES  TOTAL     MINIMUM  MAXIMUM  MEDIAN  AVERAGE   DEVIATION  
REAL-TIME     50000    4.917     0        0.005    0       0.000098  0.0003     
RUN-TIME      50000    2.67      0        0.01     0       0.000053  0.000729   
BYTES-CONSED  50000    30377664  544      608      608     607.5533  3.306255   
NIL

Results for clx-cleaning on ecl 16.1.3/OpenBSD/i5-6300U CPU @ 2.40GHz

CL-USER> (bench/create-map-unmap-destroy-window)
-             SAMPLES  TOTAL      MINIMUM  MAXIMUM  MEDIAN  AVERAGE    DEVIATION  
REAL-TIME     10000    25.292     0.001    0.049    0.002   0.002529   0.001228   
RUN-TIME      10000    20.87      0        0.04     0       0.002087   0.004178   
BYTES-CONSED  10000    995616976  99552    180144   99552   99561.695  822.3482   
NIL
CL-USER> (bench/draw-rectangle-red)
-             SAMPLES  TOTAL    MINIMUM  MAXIMUM  MEDIAN  AVERAGE   DEVIATION  
REAL-TIME     50000    0.568    0        0.001    0       0.000011  0.000106   
RUN-TIME      50000    0.45     0        0.01     0       0.000009  0.0003     
BYTES-CONSED  50000    4000480  80       560      80      80.0096   2.146604   
NIL
CL-USER> (bench/draw-rectangle-random)
-             SAMPLES  TOTAL     MINIMUM  MAXIMUM  MEDIAN  AVERAGE    DEVIATION  
REAL-TIME     50000    6.341     0        0.023    0       0.000127   0.000362   
RUN-TIME      50000    4.18      0        0.02     0       0.000084   0.000913   
BYTES-CONSED  50000    68777792  1312     1376     1376    1375.5558  3.342024   
NIL

CCL

Results for clx-master on ccl 1.11.5/Linux/Xeon(R) CPU E5-2680 v3 @ 2.50GHz

? (bench/create-map-unmap-destroy-window)
-          SAMPLES  TOTAL      MINIMUM   MAXIMUM   MEDIAN    AVERAGE  DEVIATION  
REAL-TIME  10000    67.69784   0.004886  0.137855  0.005745  0.00677  0.005232   
RUN-TIME   10000    15.900752  0         0.005355  0.001606  0.00159  0.00037    
NIL
? (bench/draw-rectangle-red)
-          SAMPLES  TOTAL     MINIMUM   MAXIMUM   MEDIAN    AVERAGE   DEVIATION  
REAL-TIME  50000    0.161066  0.000001  0.002584  0.000003  0.000003  0.000013   
RUN-TIME   50000    0.160121  0         0.002749  0         0.000003  0.000031   
NIL
? (bench/draw-rectangle-random)
-          SAMPLES  TOTAL      MINIMUM   MAXIMUM   MEDIAN    AVERAGE   DEVIATION  
REAL-TIME  50000    14.490456  0.000228  0.004392  0.000284  0.00029   0.000043   
RUN-TIME   50000    2.402451   0         0.004372  0         0.000048  0.000128   
NIL

Results for clx-cleaning on ccl 1.11.5/Linux/Xeon(R) CPU E5-2680 v3 @ 2.50GHz

? (bench/create-map-unmap-destroy-window)
-          SAMPLES  TOTAL      MINIMUM   MAXIMUM   MEDIAN    AVERAGE   DEVIATION  
REAL-TIME  10000    66.04906   0.005058  0.209647  0.006353  0.006605  0.003848   
RUN-TIME   10000    22.891577  0         0.006494  0.002305  0.002289  0.000354   
NIL
? (bench/draw-rectangle-red)
-          SAMPLES  TOTAL     MINIMUM   MAXIMUM   MEDIAN    AVERAGE   DEVIATION  
REAL-TIME  50000    0.188759  0.000001  0.002756  0.000004  0.000004  0.000016   
RUN-TIME   50000    0.191405  0         0.002988  0         0.000004  0.000031   
NIL
? (bench/draw-rectangle-random)
-          SAMPLES  TOTAL      MINIMUM   MAXIMUM   MEDIAN    AVERAGE   DEVIATION  
REAL-TIME  50000    16.107603  0.000241  0.007279  0.000316  0.000322  0.000056   
RUN-TIME   50000    4.077315   0         0.006887  0.000075  0.000082  0.000149   
NIL

Conclusion

bench/create-map-unmap-destroy-window seems detrimental for ECL. Otherwise, hope this help ;-)

[dkochmanski]

it doesn't look to bad to me, I'm going to merge this PR. Did you run CLX test suite? no unexpected failures?

[mgi]

Daniel Kochmanski notifications@github.com writes:

it doesn't look to bad to me, I'm going to merge this PR. Did you run CLX test suite? no unexpected failures?

No, I didn't. I'll do that on sbcl/ecl/ccl on the cleaning branch tomorrow and report here. -- Manuel Giraud

[mgi]

Hi, here are the results of the fiasco test:

SBCL clx-cleaning

CL-USER> (xlib-test:xlib-all-tests)
................................................................................
............................................
#<test-run of XLIB-ALL-TESTS: 4 tests, 124 assertions, 0 failures in 0.03 sec>

ECL clx-cleaning

CL-USER> (xlib-test:xlib-all-tests)
................................................................................
............................................
#<test-run of XLIB-ALL-TESTS: 4 tests, 124 assertions, 0 failures in 0.01 sec>

CCL clx-cleaning

? (xlib-test:xlib-all-tests)
................................................................................
............................................
#<test-run of XLIB-ALL-TESTS: 4 tests, 124 assertions, 0 failures in 0.011509 sec>

[dkochmanski]

Merged, thanks!

[mgi]

Wow, thanks!

[JMC-design]

Somehow I think there should have been more consulting on this. I mean seriously, are people even looking at the run times in the test results? Bytes-consed in random rectangles when you're changing just ONE colour? The benefit of structures, when properly defined, is we can just copy them into the buffer without having to do expensive slot access to extract the values. This will be more of an issue when switching to mit-shm(at least on sbcl), and will be a factor in accessing buffer data for graphics. As the lowest level interface we have to the window system it makes sense to have it as fast and clean as possible. I'd like to have mit-shm,Present, and DRI3(damn nvidia) working shortly so we can have gpu buffers with seamless transitions to and from IMAGEs without expensive reads, it seems to be working in the opposite direction to make the core protocol that affects everything slower..

[JMC-design]

Not that I'm against higher level functionality being available through CLOS classes. Example https://youtu.be/Xbfj56H4BGY

[dkochmanski]

This PR was waiting for months now, it would be better to raise concerns before it gets merged. If you provide a reproducible benchmark which shows, that structures are indeed faster in a noticeable way, please share, that would be a good argument to revert it. Otherwise, we'll keep it as it is now.

[JMC-design]

The benchmark posted here clearly demonstrates that structures are faster. Draw-rectangle-random makes 1 slot access (an gc's are cached if there's no change) and 2 class allocations within the timing code and consing has doubled in EVERY single case! I'm not sure what more evidence you need. I did not expect to wake up today and find this merged when the issue of performance came up and there is clearly shown a performance issue in the benchmarks.

[dkochmanski]

You are right, it demonstrates that they are slightly faster. Given the number of repetitions though it is not a big overhead. OTOH it makes code easier to maintain. As @PuercoPop mentioned "normal" application will be most likely I/O bound. That's why I've mentioned "noticeable way". Having things being standard-class makes it easier to debug and read the code.

Keep in mind, that "maintainability" factor is important here – CLX doesn't have many contributors and I can devote only a fraction of my time to it. As you may see in this thread I had my doubts about merge too, but unless it really impacts a practical use case I'll stick to the current version.

Since I don't have a grand development plan for CLX the rule of a thumb is: whoever is willing to devote time and skill to this project, has a good argumentation for a change (cleanup, bugfix, feature) and is motivated enough to fix the issues (like regressions, peer review remarks, supply additional material like a benchmark) decides where the project goes. I'm happy to give up on maintainership of CLX if someone is willing to do that in a responsible way (i.e has a good reputation among the community with a proven record of FLOSS contributions).

[PuercoPop]

I'm happy to give up on maintainership of CLX

Please don't. It is great to have a responsive mantainer in this project.

Thanks you for taking the time to mantain CLX