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

Support durations for TTL #2

Closed
WebFreak001 opened this issue Jan 13, 2019 · 8 comments
Closed

Support durations for TTL #2

WebFreak001 opened this issue Jan 13, 2019 · 8 comments

Comments

@WebFreak001
Copy link

The TTL settings for cache entries should support core.time : Duration, which makes the code more readable (5.seconds, 10.minutes) and also supports more accurate values than seconds.

You could also replace the time calls with MonoTime (from core.time) which can give you durations relative to two MonoTime values

@ikod
Copy link
Owner

ikod commented Jan 14, 2019 via email

@WebFreak001
Copy link
Author

the Duration struct and all its functions are @safe (because it is just a long of nanoseconds which doesn't do anything else than providing some convenience methods), MonoTime.currTime is @trusted so I think it will all work fine

@ikod
Copy link
Owner

ikod commented Jan 14, 2019

My fault.
Fix will take few days.
Thanks for hint and report!

@ikod
Copy link
Owner

ikod commented Jan 15, 2019

Hello @WebFreak001 ,

It turns out that MonoTime.currTime is much slower than time() (see below), so current plan is: to use Duration in API calls to increase readability, but internally use only one second resolution for TTL and use fast time() call to trace expiration. It seems that 1 second resolution for ttl is enough.

import std.stdio;
import core.time;
import core.stdc.time;
import std.datetime.stopwatch;
import std.algorithm;

void tt() {
    time(null);
}
void mt() {
    MonoTime.currTime;
}

void main()
{
    auto r = benchmark!(tt, mt)(100000);
    r.each!writeln;
}

give output (https://run.dlang.io/is/jiMvs4)

732 μs and 9 hnsecs
455 ms, 533 μs, and 3 hnsecs

Any objections?

@WebFreak001
Copy link
Author

WebFreak001 commented Jan 15, 2019

MonoTime never runs backwards when changing the system clock or entering/leaving DST - time can run backwards in this regard and shouldn't be used for things like TTLs. If you want to use C APIs you should rather use clock_gettime(CLOCK_MONOTONIC, ...) (on linux) which is basically what MonoTime already wraps though.

You can use MonoTimeImpl!(ClockType.coarse) which will use a less exact monotime but still only run forwards (could be like milliseconds instead of hnsecs)

How often are the time functions called anyway? Would it even make any performance impact?

@ikod
Copy link
Owner

ikod commented Jan 16, 2019

Hello, @WebFreak001

I pushed all changes to master.
TTL accept durations, also all expiration checks provided against MonoTime!(ClockType.coarse).

How often are the time functions called anyway? Would it even make any performance impact?

time functions called on each put iff any kind of ttl enabled, and on get if stored entry for given key have non-zero expiration time.

Yes, there is some performance impact, but we can't avoid it.

Code

import std.stdio;
import std.algorithm;
import std.datetime.stopwatch;

import cachetools.containers;
import cachetools;

void main(string[] args)
{
    CompressedList!string words;

    string fn = args[1];
    // fill list of words from file
    auto f = File(fn, "r");
    foreach(word; f.byLine.map!splitter.joiner) {
        words.insertBack(word.idup);
    }
    f.close();

    // run words through cache and count hits
    auto cache = new Cache2Q!(string, int);
    cache.size = 1024;
    cache.ttl(1.seconds);
    int hits;
    void fun() {
        foreach(w; words.range()) {
            auto v = cache.get(w);
            if ( !v.isNull  )
            {
                hits++;
                continue;
            }
            cache.put(w, 1);
        }
    }
    benchmark!fun(1).each!writeln;
}

produce next timings (ldc2, -release):
When ttl disabled:

408 ms, 923 μs, and 3 hnsecs

When ttl enabled:

444 ms, 135 μs, and 1 hnsec

If everything is ok, I'll close this issue.
Thanks!

@WebFreak001
Copy link
Author

You could cache the value of MonoTime.currTime in the cache functions right? There is not a lot of delaying logic inbetween the calls and a difference of a few usecs wouldn't make it fatal I think, considering before seconds were the only supported accuracy.

Otherwise the changes seem to work good 👍

@ikod
Copy link
Owner

ikod commented Jan 16, 2019

Ok, I'm closing this. Would like to fix remove safety restrictions.
Thanks for report this issue.

@ikod ikod closed this as completed Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants