Performance improvement?

83 views
Skip to first unread message

Eric Daniel

unread,
Dec 13, 2010, 1:34:18 AM12/13/10
to Quicklisp
I tried out quicklisp for the first time yesterday, and I have it say
I'm pretty impressed.

One thing I noticed was that loading a system, even after it's been
compiled, can take several seconds. For instance:

CL-USER> (time (asdf:load-system "iolib"))
Evaluation took:
7.631 seconds of real time
7.292456 seconds of total run time (6.956435 user, 0.336021 system)
[ Run times consist of 0.652 seconds GC time, and 6.641 seconds non-
GC time. ]
95.56% CPU
76 forms interpreted
14,148,640,909 processor cycles
300,376,840 bytes consed

This is with SBCL 1.0.40, on x86-32.

Profiling points to ql-dist:find-asdf-system-file, which is called 76
times and
takes about 0.1 second to run each time. Most of that time is spent
initializing a newly-created ql-dist:dist instance.

What do you think of this change:

(defparameter *cached-dists-from-file* (make-hash-table :test 'equal))

(defun make-dist-from-file (file)
"Load dist info from FILE and use it to create a dist instance."
(destructuring-bind (&optional dist timestamp)
(gethash file *cached-dists-from-file*)
(let ((file-timestamp (file-write-date file)))
(unless (and dist
(>= timestamp file-timestamp))
(setf dist
(let ((initargs (config-file-initargs file)))
(apply #'make-instance 'dist :local-distinfo-file file
initargs)))
(setf (gethash file *cached-dists-from-file*)
(list dist file-timestamp)))
dist)))

Or is it important that the dist instance be created from scratch each
time?

Eric Daniel

Zach Beane

unread,
Dec 13, 2010, 8:58:10 AM12/13/10
to quic...@googlegroups.com
Eric Daniel <eric....@gmail.com> writes:

There's a QL-DIST:WITH-CONSISTENT-DISTS macro that takes a snapshot of
the state of the dists once and reuses it for the dynamic contour of the
macro. I use it in QL:QUICKLOAD to avoid reloading dist information
multiple times. I would normally use QL:QUICKLOAD instead of
ASDF:LOAD-SYSTEM to load stuff.

I do generally want to go against the filesystem as much as
possible. I've been burned pretty frequently by caches that get out of
sync with the state of the filesystem (e.g. asdf's). However, I don't
want an ASDF:FIND-SYSTEM to be unnecessarily wasteful. I'll see what I
can do to make it faster.

Zach

Eric Daniel

unread,
Dec 14, 2010, 12:01:47 AM12/14/10
to Quicklisp
On Dec 13, 5:58 am, Zach Beane <x...@xach.com> wrote:
> There's a QL-DIST:WITH-CONSISTENT-DISTS macro that takes a snapshot of
> the state of the dists once and reuses it for the dynamic contour of the
> macro. I use it in QL:QUICKLOAD to avoid reloading dist information
> multiple times. I would normally use QL:QUICKLOAD instead of
> ASDF:LOAD-SYSTEM to load stuff.
>

Thanks for the hint. I should have read the docs more closely! BUT...

CL-USER> (time (ql:quickload :iolib))

still takes 7 seconds.

It looks like the multiple dist instance creations occur inside
QL::APPLY-LOAD_STRATEGY.
Changing it to this:

(defun apply-load-strategy (strategy)
(map nil 'ensure-installed (quicklisp-releases strategy))
(call-with-macroexpand-progress
(with-consistent-dists
(lambda ()
(format t "~&; Loading ~S~%" (name strategy))
(asdf:oos 'asdf:load-op (name strategy) :verbose nil)))))

does the trick. This looks pretty safe to me, unless QL-DIST:FIND-ASDF-
SYSTEM-FILE
(by way of the ASDF:LOAD-OP operation) needs to re-read the dists at
any point.

Eric

Eric Daniel

unread,
Dec 14, 2010, 12:05:56 AM12/14/10
to Quicklisp
> (defun apply-load-strategy (strategy)
>   (map nil 'ensure-installed (quicklisp-releases strategy))
>   (call-with-macroexpand-progress
>    (with-consistent-dists
>      (lambda ()
>        (format t "~&; Loading ~S~%" (name strategy))
>        (asdf:oos 'asdf:load-op (name strategy) :verbose nil)))))

Scratch that. New version:

(defun apply-load-strategy (strategy)
(map nil 'ensure-installed (quicklisp-releases strategy))
(call-with-macroexpand-progress
(lambda ()
(format t "~&; Loading ~S~%" (name strategy))
(with-consistent-dists
(asdf:oos 'asdf:load-op (name strategy) :verbose nil)))))

Eric

Zach Beane

unread,
Dec 14, 2010, 6:48:37 AM12/14/10
to quic...@googlegroups.com
Eric Daniel <eric....@gmail.com> writes:

> On Dec 13, 5:58 am, Zach Beane <x...@xach.com> wrote:
>> There's a QL-DIST:WITH-CONSISTENT-DISTS macro that takes a snapshot of
>> the state of the dists once and reuses it for the dynamic contour of the
>> macro. I use it in QL:QUICKLOAD to avoid reloading dist information
>> multiple times. I would normally use QL:QUICKLOAD instead of
>> ASDF:LOAD-SYSTEM to load stuff.
>>
>
> Thanks for the hint. I should have read the docs more closely! BUT...
>
> CL-USER> (time (ql:quickload :iolib))
>
> still takes 7 seconds.
>
> It looks like the multiple dist instance creations occur inside
> QL::APPLY-LOAD_STRATEGY.

[snip]

D'oh! Thanks for the catch. I'll make sure to update it to avoid
redundant dist creation.

Zach

Zach Beane

unread,
Dec 14, 2010, 10:32:26 PM12/14/10
to quic...@googlegroups.com
I updated the client to wrap all of ql:quickload in
with-consistent-dists. You can get the update by using
(ql:update-client). Things are much faster now. Thanks for the report!

Zach

Mihai Călin Bazon

unread,
Dec 15, 2010, 3:01:20 AM12/15/10
to quic...@googlegroups.com
I noticed this slowness too but I blamed it on SBCL, as I upgraded it around the time I first installed Quicklisp.  Now it's a lot better, thanks for the quick fix! :-)

-Mihai
--
Mihai Bazon,
http://mihai.bazon.net/blog

Nikodemus Siivola

unread,
May 3, 2011, 11:15:54 AM5/3/11
to quic...@googlegroups.com

The same issue still exists for LOAD-SYSTEM/REQUIRE, and consecutive
quickload calls. Consider:

(require :linedit)

(dolist (name (list-of-systems-to-quickload))
(ql:quickload name))

(ql:quickload "esrap")
(ql:quickload "s-xml")

All of these seem equally sensible modes of operation to me -- so a
persistent cache with timestamps seems like the way to go.

The one below makes

time sbcl --linedit --eval '(quit)'

take only half of what it currently takes for me.

Cheers,

-- Nikodemus

(defvar *dist-cache* nil)

(defun make-dist-from-file (file &key (class 'dist))


"Load dist info from FILE and use it to create a dist instance."

(let ((timestamp (file-write-date file))
(cell (assoc (cons file class) *dist-cache* :test #'equal)))
(if (and cell timestamp (= timestamp (cadr cell)))
(cddr cell)
(let* ((initargs (config-file-initargs file))
(dist
(apply #'make-instance class
:local-distinfo-file file
:allow-other-keys t
initargs)))
(when timestamp
(let ((cached (cons timestamp dist)))
(if cell
(setf (cdr cell) cached)
(push (cons (cons file class) cached) *dist-cache*))))
dist))))

Zach Beane

unread,
May 4, 2011, 9:01:59 AM5/4/11
to quic...@googlegroups.com
Nikodemus Siivola <nikodemu...@gmail.com> writes:

> On 15 December 2010 05:32, Zach Beane <xa...@xach.com> wrote:
>
>> I updated the client to wrap all of ql:quickload in
>> with-consistent-dists. You can get the update by using
>> (ql:update-client). Things are much faster now. Thanks for the report!
>
> The same issue still exists for LOAD-SYSTEM/REQUIRE, and consecutive
> quickload calls. Consider:
>
> (require :linedit)
>
> (dolist (name (list-of-systems-to-quickload))
> (ql:quickload name))

The first argument to ql:quickload is a list designator, so this could
also be written (ql:quickload (list-of-systems-to-quickload)).

> (ql:quickload "esrap")
> (ql:quickload "s-xml")
>
> All of these seem equally sensible modes of operation to me -- so a
> persistent cache with timestamps seems like the way to go.

Thanks, I'll think about this route.

Zach

Zach Beane

unread,
Jan 3, 2012, 9:19:22 PM1/3/12
to quic...@googlegroups.com
Nikodemus Siivola <nikodemu...@gmail.com> writes:

> On 15 December 2010 05:32, Zach Beane <xa...@xach.com> wrote:
>
>> I updated the client to wrap all of ql:quickload in
>> with-consistent-dists. You can get the update by using
>> (ql:update-client). Things are much faster now. Thanks for the report!
>
> The same issue still exists for LOAD-SYSTEM/REQUIRE, and consecutive
> quickload calls. Consider:
>
> (require :linedit)
>
> (dolist (name (list-of-systems-to-quickload))
> (ql:quickload name))
>
> (ql:quickload "esrap")
> (ql:quickload "s-xml")
>
> All of these seem equally sensible modes of operation to me -- so a
> persistent cache with timestamps seems like the way to go.
>
> The one below makes
>
> time sbcl --linedit --eval '(quit)'
>
> take only half of what it currently takes for me.

It took a year, but I finally took a stab at addressing this issue.

Instead of caching dist metadata in memory, though, I load individual
bits of metadata on-demand from a CDB file, which is acting as a fairly
fast disk-based string hash table.

For me, an (asdf:load-system "iolib") is way way faster under the new
scheme. Your actual improvement will depend a lot on your disk speed.

(ql:update-client) and a restart will fetch the new scheme.

Let me know if it helps!

Zach

Reply all
Reply to author
Forward
0 new messages