-
Notifications
You must be signed in to change notification settings - Fork 4
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
Conversation
@@ -0,0 +1,10 @@ | |||
const S = require('./dist/swipl/swipl-web'); | |||
|
|||
S().then(x => { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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})
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
and without the flag
|
@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:
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. |
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 😄 |
I took these changes and combined them with recent |
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