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 for allocation customization #2051

Open
oliverlee opened this issue Sep 18, 2024 · 6 comments
Open

Support for allocation customization #2051

oliverlee opened this issue Sep 18, 2024 · 6 comments

Comments

@oliverlee
Copy link

Is there any desire to support allocator customization? I could imagine two uses cases:

  1. running in a freestanding where the default allocator is not available. pmr could be used as a replacement in this situation.
  2. building and then simplifying an expression at compile-time.
@oliverlee
Copy link
Author

I guess this could also be much faster.

@rikardn
Copy link
Contributor

rikardn commented Sep 19, 2024

Sounds interesting. Do you have any example of how this would work?

@oliverlee
Copy link
Author

I think it's only feasible if you can also control allocation for all dependencies as well, or at least the subset in use based on the library configuration, and are willing to accept an increase in complexity.

Here is video that introduces the pmr model a bit - there's an associated playlist as well. And an article which explains the model.

Hopefully this example that orders elements in containers could provide some ideas for how it could be applied to this library: https://gcc.godbolt.org/z/sEf1nKW48

@bjodah
Copy link
Contributor

bjodah commented Sep 20, 2024

Sounds like it might be worthwhile. A compile time option that goes into symengine_config.h.in and then affects all relevant typedefs (vec_basic, set_basic, multiset_basic) etc. Downstreams project will need to be careful to actually use SymEngine::vec_basic and not just write std::vector<RCP<Basic const>> haphazardly (which at least I have done). But since this would also be opt-in, I think that is fine.

This is probably no small project though. But if you can make it work, I'd be curious to see how our benchmarks are affected when using e.g. monotonic_buffer from std::pmr.

What worries me is that any API that currently allocates would now also need an extra argument (accepting the allocator instance). That might prove to be a prohibitively intrusive.

@oliverlee
Copy link
Author

Sounds like it might be worthwhile. A compile time option that goes into symengine_config.h.in and then affects all relevant typedefs (vec_basic, set_basic, multiset_basic) etc. Downstreams project will need to be careful to actually use SymEngine::vec_basic and not just write std::vector<RCP<Basic const>> haphazardly (which at least I have done). But since this would also be opt-in, I think that is fine.

I imagine you could do this without needing generate symengine_config.h.in.

This is probably no small project though. But if you can make it work, I'd be curious to see how our benchmarks are affected when using e.g. monotonic_buffer from std::pmr.

At least now, I'm happy to help on this, but a large cost will go into ramp up. Maybe a start would be some sort of collaboration that is first focused on documenting some of the internals? That would also help other new contributors in the future.

What worries me is that any API that currently allocates would now also need an extra argument (accepting the allocator instance). That might prove to be a prohibitively intrusive.

The private APIs would need to pass around allocator instances, but the public APIs should have overloads that can use a default constructed allocator value, allow a user to ignore allocator customization unless that feature is necessary.

@oliverlee
Copy link
Author

@bjodah Here's an example of how you could use a variable template, defined at the beginning of the user program, to configure the allocator.
https://gcc.godbolt.org/z/r63G49rsE

You could use probably use boost::pmr or some other custom allocator if you really wanted to, assuming it meets the Allocator requirements. Although you are at least at C++17 if you want to use variable template specialization due to various linkage issues in C++141 and std::pmr is available from C++17. Although at that point, you might as well bump to C++20 simplify implementations by using Concepts.

Footnotes

  1. https://stackoverflow.com/questions/58495283/explicit-specialization-of-template-variable

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