CMake and better IDE support

This is my first post in this forum so I want to quickly introduce myself: I am Software Engineer at Snowflake Computing and one of the engineers that works on FDB. We have and use an older version of FDB (version 3) internally and we added improvements and bugfixes to this code base within the last two years. Our git repository contains roughly 1.5k commits.

We are planning to open source our changes as well and (hopefully) get them into the official FDB repository. This work will soon become high priority for us. We created our own fork (https://github.com/snowflakedb/foundationdb) of the repository and we’ll start pushing changes there effective immediately. For small changes we’ll simply make pull requests, but I think it makes sense to first have a discussion about the larger changes.

One of the earliest changes we made was to the build system. We use CMake to build FBD. CMake seems to be the saner choice and I think the current build system that FDB uses is an abomination. So I started to port our cmake files to the new repository. This is still work in progress. CMake downloads a current version of boost and I plan to add the other dependencies in a similar fashion. These are the things that still need to be done:

  • Packaging (rpm, deb, Windows Installer, OS X installer). This is relatively easy to do in cmake. But our version currently only does RPM (this is the only thing we need) so I did not yet write this part.
  • Running tests. We have already all the code to run simulation tests with ctest and we use that heavily to test fdb in jenkins. However, it will take some more work to open source this, as some of the scripts only make sense in our environment (for example, whenever we run tests, the resulting log files are automatically uploaded to an internal Snowflake account so we can use SQL and a custom made dashboard to inspect the results).
  • Windows: We only compile and run on Linux and OS X. Therefore we never bothered to work on Windows Support. In theory, cmake is portable. There are however a few things that will not work on Windows (like the actorcompiler that currently depends on mono and all compiler flags)
  • Currently the bindings are not compiled - you basically just get fdbserver, fdbmonitor and fdbcli. However, this will soon follow.

So this would be my first question: is this something people are interested in? We would love to get the cmake build system upstream eventually as this would reduce our maintenance cost and in general I believe that this would be beneficial for new people who would like to contribute to FDB.

Additionally we invested a lot of time in getting better tooling support in general. cmake can generate a compile_commands.json file that can be used by third-party tools like IDEs and language servers. However, this is not too useful for FDB as it will contain the files that were generated by the actorcompiler.

Instead our cmake system kicks off a simple python script that transforms this compile_commands.json file into one that contains the source files and removes the -DNO_INTELLISENSE definition. Additionally we rewrote actorcompiler.h so that it is more precise. As a result, our internal FDB source consists only of files that can be compiles without any errors. So Visual Studio Code with cquery, Vim with ycm, and Emacs with rtags (and obviously any combination of those) are working. So we have code completion, refactoring, search symbol etc.

In order to get this into the code we need to do quite some refactoring:

  • We have to turn on and off the preprocessor actorcompiler in all files. This is necessary as the macros will otherwise screw up the includes (for example state, or loop can be variable names in includes so we can’t simply delete all the state and loop tokens)
  • There is a ton of Void _ ... occurrences in the code. We changed all of these to Void _uvar.... This is sadly necessary, as the compiler does not realize that the variable _ will run out of scope. So we have a macro _uvar that will generate a unique variable name.
2 Likes

So to summarize our in-person conversation…

I’m about as eager to maintain our current build system as I am to maintain a CMake system, but replacing the build system with something vaguely sane and usable is a thing we’d need to do at some point anyway, and CMake had been raised before as the most reasonable candidate. I’m fine with having our current vcxproj file-derived build system and a CMake-based one going in parallel until the CMake one is at feature parity. @mbhaskar was a strong proponent of CMake before, and for adoption we’d probably need buy-in from other people whose work would be impacted: @alvin and @alloc. I’ll have an offline conversation with them about it though.

I’m largely motivated by the idea of getting rtags.

Void _uvar = is a hack that I’d be fine living with as well. My hope would be that we can eventually just replace Flow with C++17 resumable functions, at which point all these hacks go away anyway.

1 Like

I have no horse in the build system race.

But this

Void _ … occurrences in the code. We changed all of these to Void _uvar

is replacing something ugly with something uglier. If we are going to change something, why not make the actor compiler allow

wait( <expression of type Future<Void>> );

as a statement? That should be very easy to do, and I think having the actor compiler know about the Void type is not a big problem.

1 Like

I agree that in general, being able to ignore the return value of an actor would be nice. However, given these statements:

wait(myFuture);
waitNext(myStream.getFuture());

how do you infer the types of myFutre and myStream (and this is the simple case where we pass a variable and not a type)? Maybe I am missing something but the only wait I can think of would be to implement type deduction and that seems to be quite difficult.

I think the cleanest solution would be to have compiler support for flow. This could be through resumable functions or we could patch clang (although that is quite a project and it is one we probably won’t have time doing).

I guess what I am trying to say: I don’t really care how we solve this and I am more than willing with whatever solution people like.

Is there any reason not to allow return values to be discarded for all types, rather then just Void?

I don’t think there is. But behind the scenes the actor compiler has to generate a callback function that receives the result of the future. It might be possible to template this method so that it accepts a generic type but I am not sure. Short of that: the actorcompiler needs to know the return type. This is why the following code currently does not compile:

auto v = wait(f);
1 Like

Of course a full type inferencing async/await would be best, but is a somewhat tall order now and was more so in 2009.

What I suggested - allowing wait(f); where f has to be Future<Void> - should be very easy to implement. Just hard code the type Void for the ActorCallback and make the continuation function have an unnamed argument (Void&) rather than (Void&_). What AJ suggested would be a little nicer, but I think a lot trickier to implement efficiently. You need a Callback<T> to wait on a Future<T> even if you don’t care about the value. You could make Callback<T> inherit from UntypedCallback and change the interface to fire(void*), I guess, but yuck.

I think we could get away with type inference (although more work). I didn’t think this 100% through but I think this would be possible (and I apologize for the long post in advance, but this contains generated code):

Let’s say we have the following code:

ACTOR Future<int> foo() {
    Future<int> f = asyncCalculation();
    int count = wait(f);
    return count + 1;
}

This would currently generate to this:

namespace {
// This generated class is to be used only via foo()
template <class FooActor>
class FooActorState {
public:
    FooActorState()
    {}
 
    int a_body1(int loopDepth=0)
    {
        try {
            Future<int> f = asyncCalculation();
            if (static_cast<FooActor*>(this)->actor_wait_state < 0) return a_body1Catch1(actor_cancelled(), loopDepth);
            Future<int> __when_expr_0 = f;
            if (__when_expr_0.isReady()) { if (__when_expr_0.isError()) return a_body1Catch1(__when_expr_0.getError(), loopDepth); else return a_body1when1(__when_expr_0.get(), loopDepth); };
            static_cast<FooActor*>(this)->actor_wait_state = 1;
            __when_expr_0.addCallbackAndClear(static_cast<ActorCallback< FooActor, 0, int >*>(static_cast<FooActor*>(this)));
            loopDepth = 0;
        }
        catch (Error& error) {
            loopDepth = a_body1Catch1(error, loopDepth);
        } catch (...) {
            loopDepth = a_body1Catch1(unknown_error(), loopDepth);
        }
 
        return loopDepth;
    }
    int a_body1Catch1(Error error,int loopDepth=0)
    {
        this->~FooActorState();
        static_cast<FooActor*>(this)->sendErrorAndDelPromiseRef(error);
        loopDepth = 0;
 
        return loopDepth;
    }
    int a_body1cont1(int const& count,int loopDepth)
    {
        if (!static_cast<FooActor*>(this)->SAV<int>::futures) { count + 1; this->~FooActorState(); static_cast<FooActor*>(this)->destroy(); return 0; }
        new (&static_cast<FooActor*>(this)->SAV< int >::value()) int(count + 1);
        this->~FooActorState();
        static_cast<FooActor*>(this)->finishSendAndDelPromiseRef();
        return 0;
 
        return loopDepth;
    }
    int a_body1when1(int const& count,int loopDepth)
    {
        loopDepth = a_body1cont1(count, loopDepth);
 
        return loopDepth;
    }
    void a_exitChoose1()
    {
        if (static_cast<FooActor*>(this)->actor_wait_state > 0) static_cast<FooActor*>(this)->actor_wait_state = 0;
        static_cast<FooActor*>(this)->ActorCallback< FooActor, 0, int >::remove();
 
    }
    void a_callback_fire(ActorCallback< FooActor, 0, int >*,int value)
    {
        a_exitChoose1();
        try {
            a_body1when1(value, 0);
        }
        catch (Error& error) {
            a_body1Catch1(error, 0);
        } catch (...) {
            a_body1Catch1(unknown_error(), 0);
        }
 
    }
    void a_callback_error(ActorCallback< FooActor, 0, int >*,Error err)
    {
        a_exitChoose1();
        try {
            a_body1Catch1(err, 0);
        }
        catch (Error& error) {
            a_body1Catch1(error, 0);
        } catch (...) {
            a_body1Catch1(unknown_error(), 0);
        }
 
    }
};
// This generated class is to be used only via foo()
class FooActor : public Actor<int>, public ActorCallback< FooActor, 0, int >, public FastAllocated<FooActor>, public FooActorState<FooActor> {
public:
    using FastAllocated<FooActor>::operator new;
    using FastAllocated<FooActor>::operator delete;
    virtual void destroy() { this->Actor<int>::~Actor(); operator delete(this); }
friend struct ActorCallback< FooActor, 0, int >;
    FooActor()
         : Actor<int>(),
           FooActorState<FooActor>()
    {
        this->a_body1();
    }
    void cancel()
    {
        auto wait_state = this->actor_wait_state;
        this->actor_wait_state = -1;
        switch (wait_state) {
        case 1: this->a_callback_error((ActorCallback< FooActor, 0, int >*)0, actor_cancelled()); break;
        }
 
    }
};
}
Future<int> foo(  ) {
    return Future<int>(new FooActor());
}

Now instead of this, I would love to write this:

ACTOR Future<int> foo() {
    auto f = asyncCalculation();
    auto count = wait(f);
    return count + 1;
}

But this is problematic because the code above now heavily depends on type inference.

However, I think it would be feasable to generate this code:

namespace {
// This generated class is to be used only via foo()
template <class FooActor>
class FooActorState {
public:
    FooActorState()
    {}
 
    int a_body1(int loopDepth=0)
    {
        try {
            auto asyncCalculation();
            if (static_cast<FooActor*>(this)->actor_wait_state < 0) return a_body1Catch1(actor_cancelled(), loopDepth);
            auto __when_expr_0 = f;
            if (__when_expr_0.isReady()) { if (__when_expr_0.isError()) return a_body1Catch1(__when_expr_0.getError(), loopDepth); else return a_body1when1(__when_expr_0.get(), loopDepth); };
            static_cast<FooActor*>(this)->actor_wait_state = 1;
            __when_expr_0.addCallbackAndClear(static_cast<ActorCallback< FooActor, 0, decltype(__when_expr_0.get()) >*>(static_cast<FooActor*>(this)));
            loopDepth = 0;
        }
        catch (Error& error) {
            loopDepth = a_body1Catch1(error, loopDepth);
        } catch (...) {
            loopDepth = a_body1Catch1(unknown_error(), loopDepth);
        }
 
        return loopDepth;
    }
    int a_body1Catch1(Error error,int loopDepth=0)
    {
        this->~FooActorState();
        static_cast<FooActor*>(this)->sendErrorAndDelPromiseRef(error);
        loopDepth = 0;
 
        return loopDepth;
    }
    template<class T>
    int a_body1cont1(T const& count,int loopDepth)
    {
        if (!static_cast<FooActor*>(this)->SAV<int>::futures) { count + 1; this->~FooActorState(); static_cast<FooActor*>(this)->destroy(); return 0; }
        new (&static_cast<FooActor*>(this)->SAV< int >::value()) int(count + 1);
        this->~FooActorState();
        static_cast<FooActor*>(this)->finishSendAndDelPromiseRef();
        return 0;
 
        return loopDepth;
    }
    template<class T>
    int a_body1when1(T const& count,int loopDepth)
    {
        loopDepth = a_body1cont1(count, loopDepth);
 
        return loopDepth;
    }
    void a_exitChoose1()
    {
        if (static_cast<FooActor*>(this)->actor_wait_state > 0) static_cast<FooActor*>(this)->actor_wait_state = 0;
        static_cast<FooActor*>(this)->ActorCallback< FooActor, 0, int >::remove();
 
    }
    template<class T>
    void a_callback_fire(ActorCallback< FooActor, 0>*,T value)
    {
        a_exitChoose1();
        try {
            a_body1when1(value, 0);
        }
        catch (Error& error) {
            a_body1Catch1(error, 0);
        } catch (...) {
            a_body1Catch1(unknown_error(), 0);
        }
 
    }
    template<class T>
    void a_callback_error(ActorCallback< FooActor, 0, T >*,Error err)
    {
        a_exitChoose1();
        try {
            a_body1Catch1(err, 0);
        }
        catch (Error& error) {
            a_body1Catch1(error, 0);
        } catch (...) {
            a_body1Catch1(unknown_error(), 0);
        }
 
    }
};
// This generated class is to be used only via foo()
class FooActor : public Actor<int>, public GenericActorCallback< FooActor, 0>, public FastAllocated<FooActor>, public FooActorState<FooActor> {
public:
    using FastAllocated<FooActor>::operator new;
    using FastAllocated<FooActor>::operator delete;
    virtual void destroy() { this->Actor<int>::~Actor(); operator delete(this); }
friend struct ActorCallback< FooActor, 0, int >;
    FooActor()
         : Actor<int>(),
           FooActorState<FooActor>()
    {
        this->a_body1();
    }
    void cancel()
    {
        auto wait_state = this->actor_wait_state;
        this->actor_wait_state = -1;
        switch (wait_state) {
        case 1: this->a_callback_error((ActorCallback< FooActor, 0, int >*)0, actor_cancelled()); break;
        }
 
    }
};
}
Future<int> foo(  ) {
    return Future<int>(new FooActor());
}

GenericActorCallback would be something of the form:

template <class ActorType, int CallbackNumber>
struct ActorCallback : Callback
{
    template<class T>
    void fire(T const& value) override
    {
        static_cast<ActorType*>(this)->a_callback_fire(this, value);
    }
    void error(Error e) override
    {
        static_cast<ActorType*>(this)->a_callback_error(this, e);
    }
};

Note that Callback does not have a template argument anymore. But I honestly don’t believe that the template argument is needed anyways - the only change necessary is to template the fire method:

struct Callback
{
    Callback *prev, *next;

    template<class T>
    virtual void fire(T const&) {}
    virtual void error(Error) {}
    virtual void unwait() {}

    void insert(Callback* into)
    {
        // Add this (uninitialized) callback just after `into`
        this->prev = into;
        this->next = into->next;
        into->next->prev = this;
        into->next = this;
    }

    void insertBack(Callback* into)
    {
        // Add this (uninitialized) callback just before `into`
        this->next = into;
        this->prev = into->prev;
        into->prev->next = this;
        into->prev = this;
    }

    void insertChain(Callback* into)
    {
        // Combine this callback's (initialized) chain and `into`'s such that
        // this callback is just after `into`
        auto p = this->prev;
        auto n = into->next;
        this->prev = into;
        into->next = this;
        p->next = n;
        n->prev = p;
    }

    void remove()
    {
        // Remove this callback from the list it is in, and call unwait() on the
        // head of that list if this was the last callback
        next->prev = prev;
        prev->next = next;
        if (prev == next)
            next->unwait();
    }

    int countCallbacks()
    {
        int count = 0;
        for (Callback* c = next; c != this; c = c->next)
            count++;
        return count;
    }
};

Now having a virtual templated function looks wrong here - but it isn’t. Virtual is all about runtime, template is static. The compiler can instantiate the fire method of all children, so it will compile.

I think the biggest drawback of this approach is that compiler errors will probably be quite hard to understand. Apart from that, I don’t see any obvious reasons why it should not work, but of course I might be missing something.

I don’t think this will work. I’m afraid I don’t have time to explain why in detail, but partly because of the virtual template method in Callback, and partly because the vtbl for the concrete ActorCallback is initialized with the actor, and there is no way that I know of to make that dependent on a type inferred inside a member function of the class. The former can be somewhat straightforwardly worked around by type erasure (as I suggested for AJ’s easier suggestion), but I think the latter requires a change (and probably a cost) in what happens at runtime. You could try for example deferring construction of the callback until the wait statement, just reserving space for it in the base class, but it’s going to be ugly and very type unsafe.

I’ve posted a PR #699 that implements this, and the other changes required to the codebase to allow clang to parse un-actor-compiled .actor.cpp files, and generate a compile_commands.json to feed into other tools. I have the YouCompleteMe vim plugin now able to understand TLogServer.actor.cpp as being diagnostic-free.

1 Like