Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of nrepl-dict-get #3713

Merged
merged 5 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### Changes

- [#3714](https://github.com/clojure-emacs/cider/pull/3714): Show progress when evaluating files using `cider-load-all-files`.
- [#3713](https://github.com/clojure-emacs/cider/pull/3713): Optimize `nrepl-dict-get` and deprecate its 3-argument arity.

### Bugs fixed

Expand Down
33 changes: 21 additions & 12 deletions nrepl-dict.el
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
;;; Commentary:
;;
;; Provides functions to interact with and create `nrepl-dict's. These are
;; simply plists with an extra element at the head.
;; simply plists with an extra element at the head, and using `equal' for
;; comparison of string keys.

;;; Code:
(require 'cl-lib)
Expand All @@ -44,12 +45,11 @@
(maphash (lambda (k v) (nrepl-dict-put dict k v)) hash)
dict))

(defun nrepl-dict-p (object)
(defsubst nrepl-dict-p (object)
"Return t if OBJECT is an nREPL dict."
(and (listp object)
(eq (car object) 'dict)))
(eq (car-safe object) 'dict))

(defun nrepl-dict-empty-p (dict)
(defsubst nrepl-dict-empty-p (dict)
"Return t if nREPL dict DICT is empty."
(null (cdr dict)))

Expand All @@ -61,14 +61,23 @@ whose car is KEY. Comparison is done with `equal'."
(member key (nrepl-dict-keys dict)))

(defun nrepl-dict-get (dict key &optional default)
"Get from DICT value associated with KEY, optional DEFAULT if KEY not in DICT.
If dict is nil, return nil. If DEFAULT not provided, and KEY not in DICT,
return nil. If DICT is not an nREPL dict object, an error is thrown."
"Get from DICT value associated with KEY.
If DICT is nil, return nil.
If DICT is not an nREPL dict object, an error is thrown.

If KEY is not in DICT, return DEFAULT (if provided).
Note that the use of DEFAULT is deprecated and will be
removed in a future release."
(declare (advertised-calling-convention (dict key) "1.16"))
(when dict
(if (nrepl-dict-p dict)
(if (nrepl-dict-contains dict key)
(lax-plist-get (cdr dict) key)
default)
;; Note: The structure of the following expression avoids the
;; expensive containment check in nearly all cases, see #3717
(or (lax-plist-get (cdr dict) key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably add some comments here, so someone won't "optimize" away the optimizations in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, and rebased to resolve the conflicts

;; TODO: remove DEFAULT argument and the following clause
(when default
(and (not (nrepl-dict-contains dict key))
default)))
(error "Not an nREPL dict object: %s" dict))))

(defun nrepl-dict-put (dict key value)
Expand Down Expand Up @@ -190,7 +199,7 @@ If NO-JOIN is given, return the first non nil dict."
(t `(,dict1 ,dict2)))))


;;; Dbind
;;; Destructuring-bind of string keys
(defmacro nrepl-dbind-response (response keys &rest body)
"Destructure an nREPL RESPONSE dict.
Bind the value of the provided KEYS and execute BODY."
Expand Down
Loading