Re: possible json.el optimization: json-alist-p and json-plist-p recursion

[Ted Zlatanov]

On Thu, 13 Oct 2011 10:31:30 -0400 Stefan Monnier <address@hidden> wrote: 

>> I ran into a very deep recursion with `json-encode' because
>> `json-alist-p' is unnecessarily recursive.  This is not a bug, just
>> something that can be optimized because (it seems) Emacs Lisp doesn't do
>> good tail recursion optimization in this case.

SM> Emacs Lisp doesn't do any tail recursion optimization (to some extent
SM> because it's pretty difficult to do with a dynamically scoped languages,
SM> so hopefully we'll be able to change this for the lexically scoped
SM> dialect) and its function calls are expensive, so it does not handle
SM> recursion very well, indeed (yes, this is sad).

It's usually not a problem but in this case a "proper" recursive
approach is broken, unfortunately.  I only noticed it because I was
calling `json-encode' on large lists, which is not typical usage...

SM> I guess it's OK tho "car-safe" seems unneeded since you've just tested
SM> consp before (note that the original code didn't even check (consp p)
SM> and just signaled an error if `list' is not a proper list).

SM> Oh, and you don't need to copy `list' into `p', you can work on `list' 
directly.
...
SM> Same here about car-safe and cdr-safe, and additionally, I think you
SM> don't want to test `list' but `p' instead (tho here again, you probably
SM> want to work on `list' directly without using a `p').

Good points, I had the `car-safe' and `cdr-safe' calls left over from
earlier code, and I was copying into p unnecessarily.  This is better, I
think:

#+begin_src lisp
(defun gnus-sync-json-alist-p (list)
  "Non-null if and only if LIST is an alist."
  (while (consp list)
    (setq list (if (consp (car list))
                   (cdr list)
                 'not-alist)))
  (null list))

(defun gnus-sync-json-plist-p (list)
  "Non-null if and only if LIST is a plist."
  (while (consp list)
    (setq list (if (and (keywordp (car list))
                        (consp (cdr list)))
                   (cddr list)
                 'not-plist)))
  (null list))

#+end_src

Thanks
Ted