Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for memoizing class methods #9940

Open
dlangBugzillaToGithub opened this issue Oct 1, 2012 · 10 comments
Open

Add support for memoizing class methods #9940

dlangBugzillaToGithub opened this issue Oct 1, 2012 · 10 comments

Comments

@dlangBugzillaToGithub
Copy link

andrej.mitrovich (@AndrejMitrovic) reported this on 2012-10-01T12:33:24Z

Transfered from https://issues.dlang.org/show_bug.cgi?id=8743

CC List

  • bearophile_hugs

Description

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);
}
@dlangBugzillaToGithub
Copy link
Author

andrej.mitrovich (@AndrejMitrovic) commented on 2012-10-01T13:19:45Z

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.

@dlangBugzillaToGithub
Copy link
Author

andrej.mitrovich (@AndrejMitrovic) commented on 2012-10-01T13:23:08Z

Ahh, I've got it! Templates have their own scope, so the hash can go outside:

http://dpaste.dzfl.pl/4ba280c7

@dlangBugzillaToGithub
Copy link
Author

bearophile_hugs commented on 2012-10-01T14:00:29Z

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

@dlangBugzillaToGithub
Copy link
Author

bearophile_hugs commented on 2012-10-01T14:03:50Z

void rehash() { memo = null; }

Maybe do you mean something like this?

void rehash() { memo.rehash; }
void clear() { memo = null; }

@dlangBugzillaToGithub
Copy link
Author

andrej.mitrovich (@AndrejMitrovic) commented on 2012-10-02T02:25:42Z

(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);
}

@dlangBugzillaToGithub
Copy link
Author

bearophile_hugs commented on 2012-10-02T02:54:21Z

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

@dlangBugzillaToGithub
Copy link
Author

andrej.mitrovich (@AndrejMitrovic) commented on 2012-10-02T03:05:48Z

(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

@dlangBugzillaToGithub
Copy link
Author

andrej.mitrovich (@AndrejMitrovic) commented on 2012-10-02T03:08:44Z

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

@dlangBugzillaToGithub
Copy link
Author

andrej.mitrovich (@AndrejMitrovic) commented on 2012-10-02T03:12:47Z

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

@dlangBugzillaToGithub
Copy link
Author

andrej.mitrovich (@AndrejMitrovic) commented on 2012-10-02T03:17:42Z

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

@LightBender LightBender removed the P4 label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants