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

Move/Copy Constructor #2

Open
kdkavanagh opened this issue Aug 6, 2024 · 8 comments
Open

Move/Copy Constructor #2

kdkavanagh opened this issue Aug 6, 2024 · 8 comments

Comments

@kdkavanagh
Copy link

What would be required to enable the default move or copy constructors for jmp::static_branch? Asking because it's difficult to use these in an OOP context w/o at least one of these two

@krzysztof-jusiak
Copy link
Contributor

Could you elaborate on your use case, please? copying//moving static branch doesn't make much sense from underlying resources, it could be copied/moved but that would be misleading since static_branch has no state//fields etc. In principle it should be static constexpr.

@kdkavanagh
Copy link
Author

Now that you mention it, perhaps I'm misusing... My usecase is something like this, where I can have multiple distinct instances of MyCalculator... But yes, if its mutating the definition of MyCalculator::compute which is static/shared amongst all instances of the class, then my design wont work

struct MyCalculator {
  void compute() {
     if(!is_initialized) {
        // Do setup work
        is_initialized=true;
     }
  }
  static_branch<bool> is_initialized
}

@krzysztof-jusiak
Copy link
Contributor

I see, your use case can ma accommodated, it might need to be done a bit differently as static_branch will be likely inlined per calculate instance and then hot patching will happen on top. I think I understand though, so please allow me to experiment a bit and I'll report back here how that can be approached.

@krzysztof-jusiak
Copy link
Contributor

Okay, so there are 2 cases here to consider: if the compute call is going to be inlined a single static contsexpr branch will be enough as assemlby will have nop/jmp duplicated per each instance compute call. Though, it also requires to change the specific version only, based on the this pointer of given MyCalulator instance. That would work with the current api, however, the problematic part is if the compute call is not going to be inlined in which case copy/move would make sense. It would be easy to prevent inlining for the check and expose copy/move constructor with for example copyable_static_branch but that would mean always an unconditional jmp, which is not terrible but not ideal (still better than conditional jmp). It might be a worthy tradeoffs but maybe there is a better way, not sure? Still need some digging, I think.

@tarun-t
Copy link

tarun-t commented Aug 9, 2024

How do copy and move constructors work with multiple instances of MyCalculator class in case of no inlining? What we need in essence is a new branch for each object we have created right or am I missing something here?

@kdkavanagh
Copy link
Author

Yes, exactly - different instances per object. Not sure how that could work, after getting a better understanding of how the current impl works

@kdkavanagh
Copy link
Author

I wonder.... If the containing class has a default lambda template parameter, then each instantiation of of the object would yield a new type and therefore I think a different instantiation of the method definition? Would this be enough to accomplish this usecase?

template<auto = []{}>
struct MyCalculator {};

@tarun-t
Copy link

tarun-t commented Aug 12, 2024

That's a clever trick and could work. However, it comes with the restriction of not being able to have a container of these objects or store them in a class. We could use type erasure to address this, but comes at the cost of a virtual function dispatch. Do you agree or is there a better way of achieving the same?

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

3 participants