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

Add bundled distribution of SWIPL #8

Closed
wants to merge 5 commits into from

Conversation

jeswr
Copy link
Collaborator

@jeswr jeswr commented Dec 22, 2022

Following on from the recent discussion in #1; this modifies the build to create a swipl-bundle.js file that contains everything needed to run swipl in browser and node. I have also made this the main entry point as there currently is none.


I've calculated that the size of swipl-bundle.js is 33.5% larger than the combination of swipl-web.wasm, swipl-web.data and swipl-web.js. And it seems to run pretty fast once loaded. I personally think this is acceptable for the type of POC use-cases that I am working on.

2M swipl-web.wasm
6M swipl-web.data
1M swipl-web.js
=9M

11M swipl-bundle.js

@@ -0,0 +1,10 @@
const S = require('./dist/swipl/swipl-web');

S().then(x => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code seems to be unused and leftover from testing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That file is from the first commit of this PR - and has already been removed. This is the diff I am seeing for the whole PR

@@ -0,0 +1,59 @@
# Make console binaries runnable through Node.js.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to integrate this into swipl-devel so we won't need a patch but just different build flags. Or even better, build all 3 variants at the same time: web, node, and single-file.

Copy link
Collaborator Author

@jeswr jeswr Dec 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That file is no longer part of this PR. Instead the same outcome is achieved by making the following update to the dockerfile https://github.com/rla/npm-swipl-wasm/blob/f4eee5e8c895ca6fe93d55506000323d05d380e8/docker/Dockerfile#L69-L77

Copy link
Collaborator Author

@jeswr jeswr Dec 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even better, build all 3 variants at the same time: web, node, and single-file.

I'm not really familiar with the cmake toolchain so it is probably faster for you or @JanWielemaker to do it; rather than go through a review process with my attempts. I imagine that EmscriptenTargets.cmake should updated to look something like this; but I'm also sure there is a cleaner way of doing it.

# Make console binaries runnable through Node.js.

set(WASM_NODE_LINK_FLAGS
    -s NODERAWFS=1
    -s EXIT_RUNTIME=1)
join_list(WASM_NODE_LINK_FLAGS_STRING " " ${WASM_NODE_LINK_FLAGS})

set_target_properties(swipl PROPERTIES
		      LINK_FLAGS "${WASM_NODE_LINK_FLAGS_STRING}")

# Create the preload data containing the libraries. Note that
# alternatively we can put the library in the resource file and
# link the resource file into the main executable.

set(WASM_BOOT_FILE "${WASM_PRELOAD_DIR}/boot.prc")
add_custom_command(
    OUTPUT ${WASM_BOOT_FILE}
    COMMAND ${CMAKE_COMMAND} -E make_directory ${WASM_PRELOAD_DIR}
    COMMAND ${CMAKE_COMMAND} -E copy ${SWIPL_BOOT_FILE} ${WASM_BOOT_FILE}
    COMMAND ${CMAKE_COMMAND} -E copy_directory
			     ${SWIPL_BUILD_LIBRARY} ${WASM_PRELOAD_DIR}/library
    DEPENDS ${SWIPL_BOOT_FILE} prolog_home library_index
    VERBATIM)

add_custom_target(wasm_preload_dir DEPENDS ${WASM_BOOT_FILE})
add_dependencies(wasm_preload wasm_preload_dir)

# Build the browser-deployed binary with a bit different linker flags.

set(POSTJS ${CMAKE_CURRENT_SOURCE_DIR}/wasm/prolog.js)
set(PREJS ${CMAKE_CURRENT_SOURCE_DIR}/wasm/pre.js)

set(WASM_SHARED_LINK_FLAGS
    -s WASM=1
    -s MODULARIZE=1
    -s EXPORT_NAME=SWIPL
    -s NO_EXIT_RUNTIME=0
    -s WASM_BIGINT=1
    -s ALLOW_MEMORY_GROWTH=1
    -s EXPORTED_FUNCTIONS=@${CMAKE_SOURCE_DIR}/src/wasm/exports.json
    -s EXPORTED_RUNTIME_METHODS=@${CMAKE_SOURCE_DIR}/src/wasm/runtime_exports.json
    --pre-js ${PREJS}
    --post-js ${POSTJS})

if(MULTI_THREADED)
  list(APPEND WASM_SHARED_LINK_FLAGS
       -pthread
       -s PTHREAD_POOL_SIZE=4)
endif()

set(WASM_WEB_LINK_FLAGS
    --preload-file ${WASM_PRELOAD_DIR}@swipl)
list(APPEND WASM_WEB_LINK_FLAGS WASM_SHARED_LINK_FLAGS)

set(WASM_BUNDLE_LINK_FLAGS
    -s SINGLE_FILE
    --embed-file ${WASM_PRELOAD_DIR}@swipl)
list(APPEND WASM_BUNDLE_LINK_FLAGS WASM_SHARED_LINK_FLAGS)

join_list(WASM_WEB_LINK_FLAGS_STRING " " ${WASM_WEB_LINK_FLAGS})
join_list(WASM_BUNDLE_LINK_FLAGS_STRING " " ${WASM_WEB_LINK_FLAGS})

add_executable(swipl-web ${SWIPL_SRC})
set_target_properties(swipl-web PROPERTIES
		      LINK_FLAGS "${WASM_WEB_LINK_FLAGS_STRING}")
target_link_libraries(swipl-web libswipl)
add_dependencies(swipl-web wasm_preload)
set_property(TARGET swipl-web PROPERTY LINK_DEPENDS
	     ${POSTJS} ${PREJS})

add_executable(swipl-bundle ${SWIPL_SRC})
set_target_properties(swipl-bundle PROPERTIES
              LINK_FLAGS "${WASM_WEB_LINK_FLAGS_STRING}")
target_link_libraries(swipl-bundle libswipl)
add_dependencies(swipl-bundle wasm_preload)
set_property(TARGET swipl-bundle PROPERTY LINK_DEPENDS
         ${POSTJS} ${PREJS})

@JanWielemaker
Copy link
Member

I'm a little lost. Would it be a good option to have a CMake option that selects between the various build options?

@jeswr
Copy link
Collaborator Author

jeswr commented Dec 29, 2022

I'm a little lost. Would it be a good option to have a CMake option that selects between the various build options?

As far as I'm concerned that is overkill and it is fine to always just build all 3 variants; which would mean that the change compared to the current CMake configuration to also emit swipl-bundle.js which is produced using the same parameters as the swipl-web files with the additional flags

    -s SINGLE_FILE
    --embed-file ${WASM_PRELOAD_DIR}@swipl)

and without the flag

   --preload-file ${WASM_PRELOAD_DIR}@swipl)

@rla
Copy link
Collaborator

rla commented Dec 30, 2022

@JanWielemaker, @jeswr since we already build both browser and node variants always then lets build single-file variant too. I think that the cmake file updates proposed by Jesse look fine.

I would also like to have following updates in this package:

  • Add README example with importing directly from swipl-wasm. I see that Node example
    lacks import completely. I will update README myself. Also couple of words why we provide
    3 different modules. (@rla)
  • Test case for node and browser that loads code from -bundle version.

Should we also change the SWI-Prolog own documentation? (https://www.swi-prolog.org/pldoc/man?section=wasm-loading).

I have received very little feedback on using the WebAssembly version. Only Jesse has sent me feedback and long time ago a newer builds were asked on the forum but that's all. So it's very difficult to make decisions.

@JanWielemaker
Copy link
Member

I have received very little feedback on using the WebAssembly version.

I have no clue on the popularity. The WASM shell got about 60 users per day before holidays (now 20-30). For use with Node there are also interfaces to the real thing while the WASM version is still pretty limited and slow when compared to the real thing.

I'll have a look at this, probably next week. The more PRs the better 😄

@rla
Copy link
Collaborator

rla commented Jan 28, 2023

I took these changes and combined them with recent swipl-devel changes. I added small description to README on bundle usage and released a new version on NPM.

@rla rla closed this Jan 28, 2023
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

Successfully merging this pull request may close these issues.

None yet

3 participants