magicl icon indicating copy to clipboard operation
magicl copied to clipboard

Speed up from-list

Open jmbr opened this issue 4 years ago • 6 comments

It looks like it might be possible to speed up the from-list function:

MAGICL> (let* ((n 50000)
                     (xs (loop :for i :below n :collect (coerce i 'double-float))))
                (sb-ext:gc :full t)
                (time (magicl:from-list xs (list n) :type 'double-float))
                (sb-ext:gc :full t)
                (time (magicl:from-array (coerce xs '(array double-float (*))) (list n)))
                (values))
Evaluation took:
  1.544 seconds of real time
  1.544000 seconds of total run time (1.544000 user, 0.000000 system)
  100.00% CPU
  4,188,179,784 processor cycles
  3,578,512 bytes consed
  
Evaluation took:
  0.000 seconds of real time
  0.004000 seconds of total run time (0.004000 user, 0.000000 system)
  100.00% CPU
  1,477,558 processor cycles
  400,016 bytes consed

jmbr avatar Mar 03 '20 16:03 jmbr

Easy solution: delete from-list. 😆

notmgsk avatar Mar 04 '20 17:03 notmgsk

It would make sense to replace both by a from-sequence that appropriately coerces its argument.

However, I wonder if the slowness of from-list is a symptom of some other issue that could be improved and have positive effects in other parts of the code.

jmbr avatar Mar 04 '20 17:03 jmbr

Maybe related: https://github.com/rigetti/magicl/pull/63#discussion_r341383765

Also maybe this comment at the top of from-array

https://github.com/rigetti/magicl/pull/63/files#diff-4bbae3945a97f437320d2829a528e786R97

appleby avatar Mar 04 '20 18:03 appleby

You were spot on, @appleby. The culprit is the call to nth within into!.

jmbr avatar Mar 04 '20 18:03 jmbr

I can definitely fix the implementation of from-list as a band-aid for now but I think @jmbr is right about possibly shifting to a from-sequence constructor instead. Maybe from-array and from-list can be kept as aliases to from-sequence...

colescott avatar Mar 27 '20 22:03 colescott

What's the benchmark now @colescott ? Should this be closed?

stylewarning avatar Jan 28 '21 19:01 stylewarning