-
-
Notifications
You must be signed in to change notification settings - Fork 655
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
[cpp] Generate Typed Functions #11151
base: development
Are you sure you want to change the base?
Conversation
purely for stylistic consistency
This is awesome, and I think it's a worthwhile endeavor. It's also quite neat that you can make a change like this with a relatively small PR. Looking forward to seeing this completed! |
I just rediscovered https://github.com/HaxeFoundation/haxe/tree/development/tests/benchs while cleaning up. It's a generally useless micro-benchmark, but for something like this it's quite nice to see a direct comparison between different call kinds:
Will be nice to see if there's a before/after difference for your changes here. |
Looks good! I suppose there would be an even bigger difference if it used integers instead of strings, because then we're in boxing land. Regarding native |
I saw the face of true evil this evening, it took the form of haxe optional arguments. I've cleaned up the code handling optional arguments a bit, but my head is spinning as a result. I've now managed to apply the Seems like a lot of the native final arguments are about class variables, might be able to get away with it for functions. |
Fixed the issue with self referencing typedefs, not the most confident in my fix, but all the tests pass now. I've started updating the hxcpp implemented classes to use the new callable type. So now class closures of maths and string functions are fully typed and don't use dynamic. Arrays are the main one left, going to be a bit trickier due to the BaseArray, Array, and VirtualArray structure. The array map function will be trickier still as as C++ can't infer template types, so gencpp will need to be updated to generate extra code for arrays map callable. Cppia isn't working in some cases. The below code snippet gives a runtime error of class Main {
static function main() {
final a = []; //new Array<Dynamic>();
final f = () -> trace('hello, world!');
a.push(f);
a[0]();
}
} Interestingly if you uncomment and use that I've also been experimenting with the dynamic run functions of hxcpp's internals. Dynamic calling is handled by a series of virtual functions on the base __Run(const Array<::Dynamic> args) and __run(const Dynamic& args0);
__run(const Dynamic& args0, const Dynamic& arg1);
// etc, etc... The very large number of virtual // defined at the bottom of Array.h due to header include order...
template<class ...TArgs>
Dynamic hx::Object::__run(const TArgs& ...args)
{
auto arr = Array_obj<::Dynamic>::__new(0, sizeof...(args));
(arr->push(Dynamic(args)), ...);
return __Run(arr);
} The callable object also now marks it's Dynamic __Run(const Array<Dynamic>& inArgs) override final
{
return apply(inArgs, std::index_sequence_for<TArgs...>());
}
template<size_t... I>
Dynamic apply(const Array<Dynamic>& inArgs, std::index_sequence<I...>)
{
if constexpr (std::is_void<TReturn>())
{
_hx_run(inArgs[I] ...);
return null();
}
else
{
return _hx_run(inArgs[I] ...);
}
}
virtual TReturn _hx_run(TArgs... args) = 0; |
Managed to reduce the required C++ version down from 17 to 11, also gone through and updated all the _dyn functions in the built in hxcpp classes (String, Maths, Array, VirtualArray) to return callables instead of dynamic. The array map_dyn function was a bit of a bugger, but I managed to get it working. I.e. function main() {
final myArr = [ 1, 2, 5 ];
final myMap = myArr.map;
indirect1(myMap);
indirect2(myArr.map);
}
function indirect1(func : (f : Int->String) -> Array<String>) {
trace(func(i -> Std.string(i)));
}
function indirect2(func : (f : Int->Float) -> Array<Float>) {
trace(func(i -> i + 0.5));
} Now produces void Main_Fields__obj::main() {
::Array< int > myArr = ::Array_obj< int >::fromData( _hx_array_data_1a48cc76_1,3);
::hx::Callable< ::Array< ::String >(::hx::Callable< ::String(int)>)> myMap = myArr->map_dyn< ::String >();
::_Main::Main_Fields__obj::indirect1(myMap);
::_Main::Main_Fields__obj::indirect2(myArr->map_dyn< Float >());
} where the templated map_dyn function allows everything to be typed with no dynamic. This appears to be the only function out of all the hxcpp implemented classes which needs this special template handling to prevent dynamic cropping up. To get cppia working I had to disable the MemReference setting for objects, the reason for this is that every gets downcasted to hx::Object* and you can have a cppia script which replaces a hx::Callable's pointer with a pointer to a non callable_obj. Usually the constructors and implicit conversions take care of this, but MemReference directly re-assigns pointer addresses. With that I think everything is good to go, the haxe repo tests pass with cpp and cppia, and the hxcpp repo tests seem fine as well. I'll update the main post with the core changes made instead of it being spread across multiple comments as well as some things which might want to be done for this merge or later down the line as nice improvements. |
Updated both merges to latest changes, also started writing a post for my blog about these changes if and when they get merged. As part of that I got out the old excel to graph out those call benchmark results, I modified the benchmarks a bit to include up to four arguments and to use ints instead of strings (using int values >255 to avoid hxcpp's small int boxing cache) for maximum boxing comparison which showed some interesting results. Dynamic calling is now slower than before but I'm not too concerned as if you're explicitly using dynamic I don't think anyone gets to complain about performance. Whats more interesting is the "anon" and "class to anon" cases are still using dynamic calling. This appears to be because gencpp has some special cases for function calling on cpp.Variant (what anon types return) which goes through dynamic calling. |
I agree with that assessment and did the same on genjvm. Dynamic calls are comparatively slow there, but anon and class to anon are nicely optimized. Some of that is more about the implementation of anons itself rather than functions, because "true" anons actually end up as classes and access to them is essentially optimized as Edit: Just ran the benchmarks again on JVM, I don't remember the difference being this extreme:
|
Anon objects on hxcpp are a weird type, unless you annotate the instantiation with Just ran the jvm calling bench on my machine. Dynamic calling is slower than cpp, but not ~2% slow.
|
Had a play around with that this evening and got some interesting results. I added an extra case for I'm going to look into having |
Plotting out the graph again shows that only the dynamic case now decreases with argument count, "class to anon" and "anon" is still significantly below the others due to it performing an anon field lookup in each iteration. The right graph shows the results again but I cache the looked up anon field before the iterate to "hide" that cost and have the benchmark be entirely the call speed. In this case they go up to >95% which is what I want. |
Nice! I like seeing these graphs, it's always good to visualize progress like that. I'd like to run this PR on https://benchs.haxe.org/ too, but it would be better if CI wasn't failing like it currently does. Can we get to a state where it at least doesn't fail with |
For the cpp tests to run it will need the hxcpp pr linked in the description merged in first since this haxe merge sets the hxcpp API level to 500 which so far only exists in my hxcpp branch. The hxcpp merge ci is passing (with the exception of the unrelated flakey mac ci) which means I haven't broken backwards compatibility with my changes, so if @hughsando is happy with the overall approach and doesn't spot any issues then that should be good to merge. After that if we re-run the ci on this branch it should start to run and hopefully get all green for the first time (cpp and cppia tests all pass for me locally when using my hxcpp branch). |
Updated this branch and the matching hxcpp one to dev / master. Also managed to find a better solution to the template and partial declaration issue I came across. Since the templated |
Corresponding hxcpp pr
Something I started last year but just now decided to come back to. This attempts to have all functions be implemented as a strongly typed callable object instead of using
::Dynamic
internally.Main two reasons for this are to stop primitive types becoming boxed when used as closures arguments or with class function closures, and to allow you to have a typed function object to use when writing interop code in C++.
Key Changes
Callable Object
There is a new templated
hx::Object
subclass calledhx::Callable_obj
to represent anything which holds invokable code. Since it uses template pack parameters for arguments several of thehx::Object
functions can be overridden and marked as final in this base type to save the user or the haxe compiler needing to generate those functions. It also contains a pure virtual_hx_run
function to invoke the code.A corresponding
hx::Callable
hx::ObjectPtr
subclass has also been created. This class overrides the()
operator for nice function calling and more importantly handles generating wrapper callables to deal with all the dynamic unification haxe allows with function signatures.Dynamic Calling
The next big change is surrounding dynamic calls with
__run
and__Run
.__run
is now non-virtual and implemented as a variadic template function, using some C++ template magic we can automatically generate anArray<::Dynamic>
for passing into the still virtual__Run
.This saves classes needing to implement (either by the user or the compiler) a specific
__run
overload for dynamic calling to work correctly (albeit at the expense of an extra call / array allocation now).invoker::wrap::toDynamic
uses type traits to check at compile time the argument types and generate code for wrapping native pointers and structs in the relevant hxcpp objects for dynamic compatibility, again something which use to have to be done by the user or the compiler for objects you wanted to use as a function.Callable objects also implement and mark final
__Run
, since arguments are part of the callable signature at compile time we can again use some C++ template magic to automatically unpack theArray<::Dynamic>
to the_hx_run
call using index sequences.Due to the implementation of the above C++11 is now required for hxcpp, so the
HXCPP_CPP_11
andNO
variety defines could probably be cleaned up / removed.Further Work
GC pressure could be reduced even more if the closures for stuff like class functions were cached or maybe even pre-allocated. Could even be something like the small number boxing optimisation hxcpp currently does, where boxed integers < 255 are only cached between collections.
Some unification / cleanup could probably be done around the
cpp.Callable
type, the name alone might cause some confusion, but now there's an actual callable type this pretend one which is intercepted by gencpp might not need to exist or its functionality rolled into the new actual callable type.I have not updated anything surrounding cffi / calling functions from dynamic libraries. This is all still dynamic under the hood but could probably be updated to not be. I don't think I've ever used these methods of calling native code in hxcpp so I haven't done it for now.