core.memoize

memoizing functions that throw exceptions

Details

  • Type: Defect Defect
  • Status: In Progress In Progress
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    core.memoize 0.5.6
    clojure 1.5.1

Description

core.memoize doesn't retry if the orginal function throws an exception.
This is different to clojure.core/memoize, and looks like it's a result of using delays in core.memoize.
I think the core/memoize behaviour makes more sense - if an exception is thrown do not memoize the result, instead retry the function.
Example below.


(ns test-memo.core
(:require [clojure.core.memoize ])
(:import [java.lang.RuntimeException]))

(defn print-and-throw []
(do (println "hi") (throw (RuntimeException.))))

(def normal (memoize print-and-throw))
(def lru (clojure.core.memoize/lru print-and-throw))

user> (test-memo.core/normal)
hi
RuntimeException test-memo.core/print-and-throw (core.clj:6)
user> (test-memo.core/normal)
hi
RuntimeException test-memo.core/print-and-throw (core.clj:6)

(test-memo.core/lru)
hi
RuntimeException test-memo.core/print-and-throw (core.clj:6)
user> (test-memo.core/lru)
NullPointerException clojure.core.memoize/through*/fn-2771/fn-2772 (memoize.clj:53)

Activity

Hide
Martin Trojer added a comment -

This is caused by the following bug in delay;

http://dev.clojure.org/jira/browse/CLJ-1175

core.memoize probably shouldn't use delays, maybe revert to using atoms like (memoize).

The proposed solution in CLJ-1175 doesn't help in this case since we want to re-call the function causing the exception (again, just like memoize).

Show
Martin Trojer added a comment - This is caused by the following bug in delay; http://dev.clojure.org/jira/browse/CLJ-1175 core.memoize probably shouldn't use delays, maybe revert to using atoms like (memoize). The proposed solution in CLJ-1175 doesn't help in this case since we want to re-call the function causing the exception (again, just like memoize).
Fogus made changes -
Field Original Value New Value
Status Open [ 1 ] In Progress [ 3 ]
Hide
Fogus added a comment -

Version 0.5.7-SNAPSHOT has a potential fix in place. If you're so inclined to try it out I would appreciate any feedback that you have.
Thanks.

Show
Fogus added a comment - Version 0.5.7-SNAPSHOT has a potential fix in place. If you're so inclined to try it out I would appreciate any feedback that you have. Thanks.
Hide
Jozef Wagner added a comment -

d-lay has an unsynchronized deref, which introduces a race condition between @memory and swap!. Moreover, memory seems to store only one element, so no map is needed there. I suggest something like:

(defn ^:private d-lay [f]
  (let [sentinel (Object.)
        val-ref (atom sentinel)]
    (reify
      clojure.lang.IDeref
      (deref [this]
        (locking sentinel
          (let [val @val-ref]
            (if (identical? sentinel val)
              (let [new-val (f)]
                (reset! val-ref new-val)
                new-val)
              val)))))))
Show
Jozef Wagner added a comment - d-lay has an unsynchronized deref, which introduces a race condition between @memory and swap!. Moreover, memory seems to store only one element, so no map is needed there. I suggest something like:
(defn ^:private d-lay [f]
  (let [sentinel (Object.)
        val-ref (atom sentinel)]
    (reify
      clojure.lang.IDeref
      (deref [this]
        (locking sentinel
          (let [val @val-ref]
            (if (identical? sentinel val)
              (let [new-val (f)]
                (reset! val-ref new-val)
                new-val)
              val)))))))

People

Vote (0)
Watch (4)

Dates

  • Created:
    Updated: