Issue 8743 - Add support for memoizing class methods
Summary: Add support for memoizing class methods
Status: NEW
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: All All
: P4 enhancement
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-01 12:33 UTC by Andrej Mitrovic
Modified: 2024-12-01 16:15 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Andrej Mitrovic 2012-10-01 12:33:24 UTC
This was asked for here: http://stackoverflow.com/questions/12677498/how-do-i-use-std-functional-memoize-inside-a-class

There could be use-cases for this even though methods usually depend on internal class state. Perhaps if the method was @pure it would be safe to use it. Anyway I have an experimental implementation: 

module test;

import std.stdio;
import std.traits;
import std.typecons;
import std.datetime;

template isClassStruct(alias fun)
{
    enum bool isClassStruct = (is(fun == class) || is(fun == struct));
}

mixin template memoize(alias fun, uint maxSize = uint.max)
    if (isClassStruct!(__traits(parent, fun)))
{
    ReturnType!fun opCall(ParameterTypeTuple!fun args)
    {
        static ReturnType!fun[Tuple!(typeof(args))] memo;
        auto t = tuple(args);
        auto p = t in memo;
        if (p) return *p;
        static if (maxSize != uint.max)
        {
            if (memo.length >= maxSize) memo = null;
        }
        
        mixin("auto r = this." ~ __traits(identifier, fun) ~ "(args);");
        memo[t] = r;
        return r;
    }    
}

class A 
{
    int slowFunc(int a, int b) 
    { 
        int result;
        foreach (_; 0 .. 1024)
        {
            result += a;
            result += b;
        }
        return result;
    }
    
    mixin memoize!slowFunc fastFunc;
}

enum CallCount = 2048;

void main() 
{
    A a = new A;
    
    auto sw1 = StopWatch(AutoStart.yes);
    foreach (x; 0 .. CallCount)
    {
        a.slowFunc(100, 100);  // 11232 usecs
    }
    sw1.stop();
    writeln(sw1.peek.usecs);
    
    auto sw2 = StopWatch(AutoStart.yes);
    foreach (x; 0 .. CallCount)
    {
        a.fastFunc(100, 100);  // 302 usecs
    }
    sw2.stop();
    writeln(sw2.peek.usecs);
}
Comment 1 Andrej Mitrovic 2012-10-01 13:19:45 UTC
Here's another one that has rehashing ability: http://dpaste.dzfl.pl/abb6086f

But it comes with a runtime cost of a boolean check. I'd love to be able to make rehash a compile-time parameter, but I can't seem to make opCall work with it. Even if it did work I'd have to figure out a way to store the hash outside of the template since each template instance has it's own internal hash.
Comment 2 Andrej Mitrovic 2012-10-01 13:23:08 UTC
Ahh, I've got it! Templates have their own scope, so the hash can go outside:

http://dpaste.dzfl.pl/4ba280c7
Comment 3 bearophile_hugs 2012-10-01 14:00:29 UTC
(In reply to comment #2)
> Ahh, I've got it! Templates have their own scope, so the hash can go outside:
> 
> http://dpaste.dzfl.pl/4ba280c7

I suggest to put a copy of it in Bugzilla (as attach or just pasting it here), so it's readable even if dpaste is down, or it deletes this paste, etc.
Comment 4 bearophile_hugs 2012-10-01 14:03:50 UTC
void rehash() { memo = null; }

Maybe do you mean something like this?

void rehash() { memo.rehash; }
void clear() { memo = null; }
Comment 5 Andrej Mitrovic 2012-10-02 02:25:42 UTC
(In reply to comment #4)
> void rehash() { memo = null; }
> 
> Maybe do you mean something like this?
> 
> void rehash() { memo.rehash; }
> void clear() { memo = null; }

Absolutely. I've used the wrong term here. Here's the snippet:

module test;

import std.stdio;
import std.traits;
import std.typecons;
import std.datetime;

template isClassStruct(alias fun)
{
    enum bool isClassStruct = (is(fun == class) || is(fun == struct));
}

mixin template memoize(alias fun, uint maxSize = uint.max)
    if (isClassStruct!(__traits(parent, fun)))
{
    ReturnType!fun[Tuple!(ParameterTypeTuple!fun)] memo;
    
    void rehash() { memo.rehash(); }
    void clear() { memo = null; }
    
    ReturnType!fun opCall(ParameterTypeTuple!fun args)
    {
        auto t = tuple(args);
        auto p = t in memo;
        if (p) return *p;
        static if (maxSize != uint.max)
        {
            if (memo.length >= maxSize) memo = null;
        }
        
        mixin("auto r = this." ~ __traits(identifier, fun) ~ "(args);");
        memo[t] = r;
        return r;
    }
}

class A 
{
    int field;
    
    int slowFunc(int a, int b) 
    { 
        int result;
        foreach (_; 0 .. 1024)
        {
            result += a;
            result += b;
        }
        return result + field;
    }
    
    mixin memoize!slowFunc fastFunc;
}

enum CallCount = 2048;

void main() 
{
    A a = new A;

    int z = a.fastFunc(100, 100);
    writeln(z);
    a.field = 50;
    a.fastFunc.clear();
    z = a.fastFunc(100, 100);
    writeln(z);
}
Comment 6 bearophile_hugs 2012-10-02 02:54:21 UTC
(In reply to comment #5)

>     enum bool isClassStruct = (is(fun == class) || is(fun == struct));

No need for extra parentheses:

enum bool isClassStruct = is(fun == class) || is(fun == struct);



>     void rehash() { memo.rehash(); }

I think () aren't needed, even with -property.


>     int slowFunc(int a, int b)

If the arguments are constant it doesn't work:
int slowFunc(in int a, in int b)


>     mixin memoize!slowFunc fastFunc;

This mixin template is useful. A disadvantage is that it doesn't follow the API (usage) of the Phobos memoize. So maybe it needs a different name, like memoizeMethod, or something.
Comment 7 Andrej Mitrovic 2012-10-02 03:05:48 UTC
(In reply to comment #6)
> If the arguments are constant it doesn't work:
> int slowFunc(in int a, in int b)

Seems to be a new bug related to Tuple: Issue 8746
Comment 8 Andrej Mitrovic 2012-10-02 03:08:44 UTC
(In reply to comment #6)
> This mixin template is useful. A disadvantage is that it doesn't follow the API
> (usage) of the Phobos memoize. So maybe it needs a different name, like
> memoizeMethod, or something.

Yes. I couldn't make it have the same syntax because of the requirement of the `this` reference.
Comment 9 Andrej Mitrovic 2012-10-02 03:12:47 UTC
(In reply to comment #8)
> (In reply to comment #6)
> > This mixin template is useful. A disadvantage is that it doesn't follow the API
> > (usage) of the Phobos memoize. So maybe it needs a different name, like
> > memoizeMethod, or something.
> 
> Yes. I couldn't make it have the same syntax because of the requirement of the
> `this` reference.

Also I'm unsure about recursive calls. Existing memoize allows you to recursively call a memoized function.
Comment 10 Andrej Mitrovic 2012-10-02 03:17:42 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #6)
> > > This mixin template is useful. A disadvantage is that it doesn't follow the API
> > > (usage) of the Phobos memoize. So maybe it needs a different name, like
> > > memoizeMethod, or something.
> > 
> > Yes. I couldn't make it have the same syntax because of the requirement of the
> > `this` reference.
> 
> Also I'm unsure about recursive calls.

And I think I've uncovered yet another bug:

class A 
{
    int slowFunc(int a, int b) 
    { 
        int result;
        
        /* Error: function expected before (), not mixin memoize!(slowFunc) fastFunc; */
        if (a > 1)
            return fastFunc(a-1, b);
        
        return result;
    }

    mixin memoize!slowFunc fastFunc;
}

If I replace 'return fastFunc(a-1, b);' with 'return this.fastFunc(a-1, b)' it works.
Comment 11 dlangBugzillaToGithub 2024-12-01 16:15:49 UTC
THIS ISSUE HAS BEEN MOVED TO GITHUB

https://github.com/dlang/phobos/issues/9940

DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB