Skip to content

Split this repo into evision_beam and evision #210

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

Open
cocoa-xu opened this issue Jul 28, 2023 · 9 comments
Open

Split this repo into evision_beam and evision #210

cocoa-xu opened this issue Jul 28, 2023 · 9 comments

Comments

@cocoa-xu
Copy link
Owner

Following suggestions from @josevalim with some changes

Step 1

Split the repository in two directories:

  • evision_beam. The goal of this project is to:
    • Contains all C files and all Python scripts
    • Support precompilation
    • Define all NIFs
    • Define artefact files (those are files generated by the Python scripts, with the documentation and everything necessary to autogenerate the Elixir libraries) (the format of those files can be either JSON or file:consult/2 format)
  • evision
    • Depends on the above
    • Use the artefact files to generate all Elixir APIs

Step 2

We can now break evision into more projects. evision itself should only keep the core shared by all libraries (Evision, Evision.Mat, backends, and so on). Again, it still uses the artefact files to generate the relevant APIs. But you can now break Evision.CUDA and Evision.Zoo into their own libraries. They will also read from the artefact files and generate code, it is just they encapsulate that particular part on their own.

@cocoa-xu
Copy link
Owner Author

$ rebar3 compile
===> Verifying dependencies...
===> Upgrading cc_precompiler v0.1.8
===> Upgrading dll_loader_helper v1.0.0
===> Upgrading elixir_make v0.7.6
===> Compiling elixir_make
===> Error building application elixir_make:
     No project builder is configured for type mix

Perhaps I also need to split dll_loader_helper into dll_loader_helper_beam and dll_loader_helper

@josevalim
Copy link
Contributor

Is dll_loader_helper precompiled? Do we actually need on evision? Iirc it is only a requirement when we need to mangle the search paths, no?

@cocoa-xu
Copy link
Owner Author

cocoa-xu commented Aug 1, 2023

Yeah I think so? At least we need it for finding CUDA drivers on Windows.

@josevalim
Copy link
Contributor

@cocoa-xu given the code for ddl_loader_helper is small and mostly done, maybe we just bundle it as part of this library? I am not sure if it is worth splitting dll_loader_helper into two libraries.

@cocoa-xu
Copy link
Owner Author

cocoa-xu commented Aug 1, 2023

@cocoa-xu given the code for ddl_loader_helper is small and mostly done, maybe we just bundle it as part of this library? I am not sure if it is worth splitting dll_loader_helper into two libraries.

Ah, no worries! It's been done during the last weekend, and it probably would also benefit other Erlang users in similar situations.

@josevalim
Copy link
Contributor

@cocoa-xu the issue is that evision_beam has to depend on dll_loader_helper_beam and it can't be overridden. However, if only dll_loader_helper is precompiled, it means we will always use the non-precompiled dll_loader_helper_beam, so it might as well be a single library?

@cocoa-xu
Copy link
Owner Author

cocoa-xu commented Aug 1, 2023

Oh for that part, now dll_loader_helper_beam is precompiled, and is a dependency of dll_loader_helper. Basically, dll_loader_helper is a very simple binding to the dll_loader_helper_beam now.

The only issue now is probably I need to try downloading precompiled ones in rebar.config

  {"win32", compile, "nmake Makefile.win"}

https://github.com/cocoa-xu/dll_loader_helper/blob/9d05b900ad2a6901313d33952ded259ce869bf83/dll_loader_helper_beam/rebar.config#L7

which is basically, do these things without using nmake

    @ erlc "precompiled.erl" && \
	  erlc "checksum.erl" && \
	  erl -noshell -s init stop -s precompiled install_precompiled_binary_if_available || \
    ( 
      nmake DLL_LOADER_HELPER_BEAM_USE_PRECOMPILED=false && \
      if exist "erl_crash.dump" del /f erl_crash.dump
    )

https://github.com/cocoa-xu/dll_loader_helper/blob/9d05b900ad2a6901313d33952ded259ce869bf83/dll_loader_helper_beam/Makefile.win#L36-L42

@josevalim
Copy link
Contributor

What if we keep it as a single project and with both rebar3 and mix.exs files? Then we do the precompilation bits only in Elixir for now? The project can have both lib/dll_loader_helper.ex and src/dll_loader_helper.erl but I think we can deprecate the Elixir one.

My concern is all about maintainability. We break the code apart, we duplicate parts of cc_precompiler and so on :(

@cocoa-xu
Copy link
Owner Author

Note to myself

Before doing this, we have to figure out a way to address the inheritance issue reported in #223.

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

2 participants