Performance improvement?
Eric Daniel
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
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
> 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.
>
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
> (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)))))
(defun apply-load-strategy (strategy)
(map nil 'ensure-installed (quicklisp-releases strategy))
(call-with-macroexpand-progress
(format t "~&; Loading ~S~%" (name strategy))
Zach Beane
> 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
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
-Mihai
Mihai Bazon,
http://mihai.bazon.net/blog
Nikodemus Siivola
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
> 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
> 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