<< Back to previous view

[CMEMOIZE-13] Reflection warning in clojure.core.memoize/snapshot Created: 14/Jan/14  Updated: 20/Mar/14

Status: Open
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Miikka Koskinen Assignee: Fogus
Resolution: Unresolved Votes: 1
Labels: None
Environment:

core.memoize 0.5.6, Clojure 1.5.1


Attachments: Text File cmemoize-13-v1.patch    
Patch: Code

 Description   

When core.memoize is used with *warn-on-reflection* set to true:

Reflection warning, clojure/core/memoize.clj:72:23 - reference to field cache can't be resolved.


 Comments   
Comment by Andy Fingerhut [ 20/Mar/14 6:49 PM ]

Patch cmemoize-13-v1.patch eliminates the reflection in fn snapshot by type-hinting the value of @cache





[CMEMOIZE-12] NPE with memoize/ttl Created: 05/Dec/13  Updated: 19/Feb/14

Status: Open
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Scott Morgan Assignee: Fogus
Resolution: Unresolved Votes: 1
Labels: bug
Environment:

org.clojure/core.memoize "0.5.6"



 Description   

I'm trying still trying to reproduce it, but I noted this in my logs if my compojure app on a subsequent request to the memoized function.

java.lang.NullPointerException
at clojure.core.memoize$through_STAR_$fn_401$fn_402.invoke(memoize.clj:53)
at clojure.lang.Delay.deref(Delay.java:33)
at clojure.core$deref.invoke(core.clj:2128)
at clojure.core.memoize$build_memoizer$fn__456.doInvoke(memoize.clj:140)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.lang.AFunction$1.doInvoke(AFunction.java:29)
at clojure.lang.RestFn.invoke(RestFn.java:436)



 Comments   
Comment by Aaron Iba [ 19/Feb/14 4:49 PM ]

I have the same exact stack trace / line numbers in my logs. I have no idea how to reproduce. I would love to get advice on how to debug this.

In my case, this is running in a web app (ring) context.

Comment by Nicola Mometto [ 19/Feb/14 6:13 PM ]

I noticed that core.memoize throws an NPE if the function memoized throws an exception.
I don't know if this is what you're running into but it might be something to look into.





[CMEMOIZE-8] memoizing functions that throw exceptions Created: 09/Jul/13  Updated: 02/Jan/14

Status: In Progress
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: davie moston Assignee: Fogus
Resolution: Unresolved Votes: 0
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)



 Comments   
Comment by Martin Trojer [ 25/Jul/13 10:31 AM ]

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).

Comment by Fogus [ 13/Aug/13 8:22 AM ]

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.

Comment by Jozef Wagner [ 02/Jan/14 4:46 PM ]

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)))))))




[CMEMOIZE-10] Odd deprecation warning in face of correct API usage? Created: 29/Nov/13  Updated: 03/Dec/13

Status: Open
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Magnar Sveen Assignee: Fogus
Resolution: Unresolved Votes: 0
Labels: documentation, errormsgs
Environment:

[org.clojure/core.memoize "0.5.6"]



 Description   

Calling

(clojure.core.memoize/ttl get-optimized-assets {} :ttl/threshold ms)

Results in this

WARNING - Deprecated construction method for lu cache; prefered way is: (clojure.core.memoize/lu function <base> <:lu/threshold num>)

This breaks my expectation in two ways:

  • If I'm using `ttl` wrong, I would expect a deprecation warning for that. Now I'm a little stuck.
  • If I'm using `ttl` right, I wouldn't expect a deprecation warning.

Now I am just confused. Am I using a deprecated API? In that case, what is the right one to use? Looking at the source code, I see

Please use clojure.core.memoize/ttl instead.

Which is what I am using, as far as I can tell.

Maybe not a very important issue, but it has me confused and unsure.



 Comments   
Comment by Magnar Sveen [ 03/Dec/13 12:12 AM ]

Turns out I got bit by the JVMs global namespace. Once I included 0.5.6, the ring-middleware-format in our stack also started using that updated version - with the wrong API.

Is it only node.js that has this dependency thing figured out properly?

Sorry about the misleading issue. I'd close it, but I don't have the rights to do so.





[CMEMOIZE-11] Investigate why build errors occur for Clojure 1.2.0 on IBM JDK 1.6 Created: 29/Nov/13  Updated: 29/Nov/13

Status: Open
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Fogus Assignee: Fogus
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The failure report at http://build.clojure.org/job/core.memoize-test-matrix/71/ has more detail. Error messages refer to keyword interning. The dependency has been updated to Clojure 1.2.1 for now.






[CMEMOIZE-9] memo-swap! is misnamed Created: 23/Sep/13  Updated: 23/Sep/13

Status: Open
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mark Engelberg Assignee: Fogus
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In order to be analogous to atoms, the current behavior of memo-swap! should be called memo-reset! since it overwrites the entire memo map. Similarly, there should be a function called memo-swap! that does an atomic swap operation on the memo cache so, for example, you can add a single value to the cache.






[CMEMOIZE-5] Race condition occasionally generates NullPointerException when using memo-ttl Created: 31/May/13  Updated: 26/Jul/13  Resolved: 17/Jun/13

Status: Resolved
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: James Reeves Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None

Patch: Code

 Description   

The build-memoizer function assumes that a lookup on the cache will never return nil, but in the case of a TTL cache, there is a small chance of this happening.

The problem occurs when the TTL cache expires between lines 129 and 130 in the build-memoizer function: https://github.com/clojure/core.memoize/blob/21d679df68044b11483ee9720911ff9e4d0ab9fd/src/main/clojure/clojure/core/memoize.clj#L130

This code snippet reproduces the issue:

(let [id (memo-ttl identity 100)]
  (dotimes [_ 1000000] (id 1)))


 Comments   
Comment by Fogus [ 03/Jun/13 1:48 PM ]

Version 0.5.4 should fix this problem. I added a regression test based on your snippet above.

Comment by Fogus [ 17/Jun/13 12:54 PM ]

Fixed in version 0.5.4+.





[CMEMOIZE-4] Dyadic memo-clear! evicts based on memoized fn args Created: 29/Jun/12  Updated: 28/Jun/13  Resolved: 28/Jun/13

Status: Resolved
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Homer Strong Assignee: Fogus
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File dyadic-memo-clear.patch    
Patch: Code

 Description   

A straightforward addition for the existing interface to the underlying cache. Useful for cases where the memoization cache need only be evicted for some inputs.

Please let me know if this is not the desired channel or format for submissions!



 Comments   
Comment by Fogus [ 17/Jun/13 12:55 PM ]

I'll work this into the next version. Thank you.

Comment by Fogus [ 28/Jun/13 7:31 AM ]

Applied to master. A new release will come soon.





[CMEMOIZE-6] assert on ttl fn value makes any callable other than a pure clojure.lang.Fn be rejected Created: 27/Jun/13  Updated: 28/Jun/13  Resolved: 28/Jun/13

Status: Resolved
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Max Penet Assignee: Fogus
Resolution: Completed Votes: 0
Labels: bug, patch

Attachments: Text File fix-assert.patch    
Patch: Code

 Description   

ttl memoize used to accept multimethods for instance, this is no longer the case since the latest version where an assert? was introduced that checks using fn?

The patch attached allows any of the following: clojure.lang.IFn, clojure.lang.AFn, java.lang.Runnable, java.util.concurrent.Callable.

(I am a registered contributor listed under "Maximilien Penet (mpenet)")



 Comments   
Comment by Fogus [ 28/Jun/13 7:17 AM ]

This is fixed on master. I will push a point-release out to Maven Central later today.

Comment by Fogus [ 28/Jun/13 7:31 AM ]

Applied to master. A new release will come soon.





[CMEMOIZE-7] broken jira link in readme Created: 27/Jun/13  Updated: 28/Jun/13  Resolved: 28/Jun/13

Status: Resolved
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Max Penet Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File readme.patch    
Patch: Code

 Description   

broken link in readme :

"* [Bug Tracker](http://dev.clojure.org/jira/browse/memoize)"



 Comments   
Comment by Fogus [ 28/Jun/13 7:16 AM ]

Thank you.





[CMEMOIZE-2] Deprecate Unk Created: 13/Dec/11  Updated: 03/Jun/13  Resolved: 03/Jun/13

Status: Resolved
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Minor
Reporter: Fogus Assignee: Fogus
Resolution: Completed Votes: 0
Labels: documentation, unk


 Description   

core.memoize was originally Unk , but its inclusion into contrib means the latter is redundant. The Unk repo should indicate the move and change to a place for docs and examples. Likewise, a formal announcement should be made.



 Comments   
Comment by Fogus [ 03/Jun/13 9:40 AM ]

Removed metadata and comment references.





[CMEMOIZE-1] Cut 0.5.0 release jar Created: 06/Dec/11  Updated: 14/Dec/11  Resolved: 14/Dec/11

Status: Resolved
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Fogus Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None


 Description   

The core.memoize functionality is inline with Unk. A release should be cut and pushed to Maven central.



 Comments   
Comment by Fogus [ 13/Dec/11 7:45 AM ]

v0.5.0 jar deployed to Maven central – awaiting its availability. Release notes outstanding.

Comment by Fogus [ 14/Dec/11 7:31 AM ]

Public announcement: http://groups.google.com/group/clojure/browse_thread/thread/340fdd5d64d4a642

Blog post: http://blog.fogus.me/2011/12/14/announcing-core-memoize-v0-5-1/

MVN Central: http://search.maven.org/#browse%7C-296669917





[CMEMOIZE-3] snapshot assumes too much about a CacheProtocol implementation Created: 13/Dec/11  Updated: 14/Dec/11  Resolved: 14/Dec/11

Status: Resolved
Project: core.memoize
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Paul Stadig Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File 3-fix-snapshot-assumptions.diff     File 3-fix-snapshot-assumptions-updated.diff    
Patch: Code and Test
Approval: Accepted

 Description   

I believe this line https://github.com/clojure/core.memoize/blob/9bdf608af8bbf4a753fc19758446dd821cb45979/src/main/clojure/clojure/core/memoize.clj#L72 assumes too much about implementations of CacheProtocol, namely that they have a field named 'cache'.

The first call to .cache makes sense, since PluggableMemoization has a 'cache' field, but the second call to .cache assumes that any implementation of CacheProtocol that PluggableMemoization wraps will have a 'cache' field.

I ran into this issue when I desired to have a cache that ignores nils, so I attempted to wrap the TTLCache with an implementation of CacheProtocol that would not cache nil values. Calls to snapshot failed since it was trying to access a 'cache' field on my NoNilCache.

I suspect that the outer call to .cache could be replaced with a call to seq, since defcache adds an implementation of seq to each cache implementation which should pull out the keys/values from the cache.

I'd be glad to work up a patch given that I'm not off base in my assumptions.



 Comments   
Comment by Fogus [ 13/Dec/11 3:08 PM ]

@Paul

The better assumption is that the underlying caches are associative. Thanks for reporting and a patch would be wonderful.

Comment by Paul Stadig [ 13/Dec/11 4:16 PM ]

Added test and removed one layer of calls to .cache. I tested this against my code base, and it fixes the issue I was seeing.

Comment by Paul Stadig [ 13/Dec/11 4:28 PM ]

Updated patch. The tests worked before by throwing an exception on failure, but I updated it by adding an is assertion.





Generated at Sun Apr 20 18:50:00 CDT 2014 using JIRA 4.4#649-r158309.