destructuring lambda lists inefficiently traverse the list of supplied keyword arguments twice, once to check if the keyword was supplied and a second time to return the value

Bug #707578 reported by Jean-Philippe Paradis on 2011-01-25
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Douglas Katzman

Bug Description

What I do:
(destructuring-bind (&key a b c d e f g h i) '(:c c :f f :h h)
  (values a b c d e f g h i))

What happens:
Here's the expansion:
(LET ((#:WHOLE1147 '(:C C :F F :H H)))
  (LET* ((#:KEYWORDS-1149 #:WHOLE1147))
                                    '(:I :H :G :F :E :D :C :B :A) 'NIL)
      (WHEN #:PROBLEM1150
    (LET* ((A
                (SB-KERNEL::LOOKUP-KEYWORD :A #:KEYWORDS-1149)
                (SB-KERNEL::LOOKUP-KEYWORD :B #:KEYWORDS-1149)
                (SB-KERNEL::LOOKUP-KEYWORD :C #:KEYWORDS-1149)
                (SB-KERNEL::LOOKUP-KEYWORD :D #:KEYWORDS-1149)
                (SB-KERNEL::LOOKUP-KEYWORD :E #:KEYWORDS-1149)
                (SB-KERNEL::LOOKUP-KEYWORD :F #:KEYWORDS-1149)
                (SB-KERNEL::LOOKUP-KEYWORD :G #:KEYWORDS-1149)
                (SB-KERNEL::LOOKUP-KEYWORD :H #:KEYWORDS-1149)
                (SB-KERNEL::LOOKUP-KEYWORD :I #:KEYWORDS-1149)
      (VALUES A B C D E F G H I))))

Here's the implementation of SB-KERNEL::KEYWORD-SUPPLIED-P:
(defun keyword-supplied-p (keyword key-list)
  (do ((remaining key-list (cddr remaining)))
      ((endp remaining))
    (when (eq keyword (car remaining))
      (return t))))

Here's the implementation of SB-KERNEL::LOOKUP-KEYWORD:
(defun lookup-keyword (keyword key-list)
  (do ((remaining key-list (cddr remaining)))
      ((endp remaining))
    (when (eq keyword (car remaining))
      (return (cadr remaining)))))

The wastage is manifest.

What I expected to happen:
SBCL should check to see if the keyword is supplied and fetch the value in one step.

SBCL version: 1.0.42
uname -a: Linux dynamorph 2.6.32-27-generic #49-Ubuntu SMP Wed Dec 1 23:52:12 UTC 2010 i686 GNU/Linux


Nikodemus Siivola (nikodemus) wrote :

I would be happy to merge a patch that does this, as long as it doesn't complicate the code.

I am, however, not likely to ever work on this myself absent a test-case where this actually shows up as a performance issue. :)

Changed in sbcl:
importance: Undecided → Wishlist
status: New → Triaged
Douglas Katzman (dougk) on 2015-06-11
Changed in sbcl:
assignee: nobody → Douglas Katzman (dougk)
Douglas Katzman (dougk) on 2015-07-02
Changed in sbcl:
status: Triaged → Fix Committed
Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers