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

RFC: Proc External Implementation Setting #3

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

distributivgesetz
Copy link

This RFC defines an alternative way to define and call external code in DM, using proc settings.

Not sure if I like the separate proc settings, I'd rather them be one, feel free to suggest a better way.

@Cyberboss
Copy link

Echoing @alexkar598
image

@moonheart08
Copy link

moonheart08 commented May 19, 2024

Echoing @alexkar598 image

My response here is very simple: JIT, static typing, compiler optimizations around external calls, and true C-ABI FFI, none of which can be done with call_ext. (call_ext is an optimization barrier when inlined, even.)

in other words call_ext is dynamically linked, this is not and can enable proper opts.

@SpaceManiac
Copy link

Idea: instead of procs with set declarations and empty bodies, identify and optimize procs that match the relevant pattern:

/proc/my_global_proc(my_arg1 as num, my_arg2 as text)
    return call_ext("constant_lib_string", "constant_function_string")(my_arg1, my_arg2)

@moonheart08
Copy link

Idea: instead of procs with set declarations and empty bodies, identify and optimize procs that match the relevant pattern:

/proc/my_global_proc(my_arg1 as num, my_arg2 as text)
    return call_ext("constant_lib_string", "constant_function_string")(my_arg1, my_arg2)

The thing is it's not a valid optimization anymore if this is done (due to call_ext semantics implying late loaded DLLs, among other issues), though this is a good polyfill to include in procs using the sets.

@distributivgesetz
Copy link
Author

distributivgesetz commented May 20, 2024

Idea: instead of procs with set declarations and empty bodies, identify and optimize procs that match the relevant pattern:

/proc/my_global_proc(my_arg1 as num, my_arg2 as text)
    return call_ext("constant_lib_string", "constant_function_string")(my_arg1, my_arg2)

Something I just realized is, since Unix and Windows both ban / in filenames we can use something like set extern_impl = "library/function" as syntax to merge it down to one set declaration

@moonheart08
Copy link

Idea: instead of procs with set declarations and empty bodies, identify and optimize procs that match the relevant pattern:

/proc/my_global_proc(my_arg1 as num, my_arg2 as text)
    return call_ext("constant_lib_string", "constant_function_string")(my_arg1, my_arg2)

Something I just realized is, since Unix and Windows both ban / in filenames we can use something like set extern_impl = "library/function" as syntax to merge it down to one set declaration

the lib string can presumably be a path, so not really.

@distributivgesetz
Copy link
Author

the lib string can presumably be a path, so not really.

i dont think they can? the reference does not mention this

im down to do set extern_impl = list("foo","bar"), because BYOND will complain either way

@ike709
Copy link
Collaborator

ike709 commented Jun 2, 2024

My main concern with this RFC is needing two separate set statements for every proc, one for the DLL and one for the function name. I'd much rather combine it into one set with some delimiter, or have some other way of only needing to set the library once.

As moony stated, this RFC has the potential for enabling optimizations down the road, but in its current state I don't think I'd sign off on it until my concern is addressed.

That being said, per the RFC process I'm sharing this RFC with the wider community and setting the normal 1 week timer. Let's see if they have ideas.

I'm also not fond of making it a list, since what would be a no-op in BYOND is now an error due to not being a constant. People could just use a wrapper macro, but I feel like there's a way we could make this work in both OD and BYOND if we could just come to a consensus on a delimiter between the library and the function name.

If everyone else thinks it'd be better to just use a list and a wrapper macro, then I'll concede the point.

Also whatever the consensus lands on, the set should have the od_ prefix in line with other OD set statements.

@ike709
Copy link
Collaborator

ike709 commented Jun 2, 2024

Apparently custom set statements aren't actually valid DM and the way DreamChecker gets around this is with its wrapper macros.

#ifdef SPACEMAN_DMM
	#define SHOULD_CALL_PARENT(X) set SpacemanDMM_should_call_parent = X
#else
	#define SHOULD_CALL_PARENT(X)
#endif

Et cetera.

So we'll need to do something similar and I'm back to being fine with the value being list(library, function) or similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants