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

feat: create a bundled build of wasm #1088

Closed
wants to merge 1 commit into from
Closed

feat: create a bundled build of wasm #1088

wants to merge 1 commit into from

Conversation

jeswr
Copy link
Contributor

@jeswr jeswr commented Jan 8, 2023

Follows up on the discussion SWI-Prolog/npm-swipl-wasm#8 to produce a swipl-bundle.js build which inlines the .wasm and .data files.

@jeswr
Copy link
Contributor Author

jeswr commented Jan 9, 2023

Another useful build is the swipl-bundle without any data file inlined. This is because that file is not used when there is another pre-loaded image being used such as eyereasoner/eye-js#21.

Would you accept another build of this nature? It would require the change in #1088 (comment); but the result is a 2.5M bundle (as opposed to 11M with the --embed-file ${WASM_PRELOAD_DIR}@swipl flag enabled).

Comment on lines +61 to +71
set(WASM_BUNDLE_LINK_FLAGS
-s SINGLE_FILE
--embed-file ${WASM_PRELOAD_DIR}@swipl)
join_list(WASM_BUNDLE_LINK_FLAGS_STRING " " ${WASM_BUNDLE_LINK_FLAGS} " " ${WASM_SHARED_LINK_FLAGS})
add_executable(swipl-bundle ${SWIPL_SRC})
set_target_properties(swipl-bundle PROPERTIES
LINK_FLAGS "${WASM_BUNDLE_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})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
set(WASM_BUNDLE_LINK_FLAGS
-s SINGLE_FILE
--embed-file ${WASM_PRELOAD_DIR}@swipl)
join_list(WASM_BUNDLE_LINK_FLAGS_STRING " " ${WASM_BUNDLE_LINK_FLAGS} " " ${WASM_SHARED_LINK_FLAGS})
add_executable(swipl-bundle ${SWIPL_SRC})
set_target_properties(swipl-bundle PROPERTIES
LINK_FLAGS "${WASM_BUNDLE_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})
set(WASM_BUNDLE_EMBEDDED_LINK_FLAGS
-s SINGLE_FILE
--embed-file ${WASM_PRELOAD_DIR}@swipl)
set(WASM_BUNDLE_LINK_FLAGS
-s SINGLE_FILE)
join_list(WASM_BUNDLE_LINK_FLAGS_STRING " " ${WASM_BUNDLE_LINK_FLAGS} " " ${WASM_SHARED_LINK_FLAGS})
join_list(WASM_BUNDLE_EMBEDDED_LINK_FLAGS_STRING " " ${WASM_BUNDLE_EMBEDDED_LINK_FLAGS} " " ${WASM_SHARED_LINK_FLAGS})
add_executable(swipl-bundle ${SWIPL_SRC})
set_target_properties(swipl-bundle PROPERTIES
LINK_FLAGS "${WASM_BUNDLE_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})
add_executable(swipl-bundle-embedded ${SWIPL_SRC})
set_target_properties(swipl-bundle-embedded PROPERTIES
LINK_FLAGS "${WASM_BUNDLE_EMBEDDED_LINK_FLAGS_STRING}")
target_link_libraries(swipl-bundle-embedded libswipl)
add_dependencies(swipl-bundle-embedded wasm_preload)
set_property(TARGET swipl-bundle-embedded PROPERTY LINK_DEPENDS
${POSTJS} ${PREJS})

@jeswr
Copy link
Contributor Author

jeswr commented Jan 15, 2023

@JanWielemaker - This is ready for review

@JanWielemaker
Copy link
Member

This is ready for review

Hope to take care of this shortly. I've a bit of a backlog on issues though 😢

@JanWielemaker
Copy link
Member

The bundle builds fine, but trying to use it in shell.html results in

TypeError: Module.preRun is undefined

What does this change to how it is supposed to be used?

@jeswr
Copy link
Contributor Author

jeswr commented Jan 22, 2023

preRun is used to perform operations before initializing SWIPL (for instance here https://github.com/eyereasoner/eye-js/blob/0cc5e11f44a042f322c6e77e3370d8b6b5d30011/scripts/generate-pvm.ts#L36-L39 it loads eye.pl before swipl -q -f eye.pl is run).

It should be able to be left undefined and in those cases just do nothing. I'm unsure why this is a new error - I don't think it would be caused by the new configuration file. In particular I did not get any errors of the sorts when using the output of this build file on commit b089616 (i.e. the commit that npm-swipl-wasm is using in the DockerFile). Have you made any changes to any other code since then that may be the culprit?

@jeswr
Copy link
Contributor Author

jeswr commented Jan 22, 2023

I tried having a quick look at this diff but could not identify anything that might be the cause on first parse.

@jeswr
Copy link
Contributor Author

jeswr commented Jan 23, 2023

I've tried (unsucessfully) to reproduce this error on this branch of npm-swipl-wasm. To test simply clone the branch and then run

yarn install
yarn run build
yarn run serve

For me the website appears to run without problem


In addition, I've tested these changes on the latest version of swipl-devel in the same branch. The npm-swipl-wasm tests run fine after the changes in #1102 are made.

However if I add a new test

it(`[${name}] ` + "should be able to preload files and generate images", async () => {
        const eye = await eyePlString();

        const Module = await SWIPL({ 
          arguments: ['-q', '-f', 'eye.pl'],
          preRun: (Module) => Module.FS.writeFile('eye.pl', eye) });
        
        queryOnce(Module, 'main', ['--image', 'eye.pvm']);

        assert.strictEqual(Module.FS.readFile('eye.pvm').length > 500, true);
      });

then it fails with

  1) SWI-Prolog WebAssembly on Node.js
       [node] should be able to preload files and generate images:
     TypeError: Module.preRun.push is not a function
      at loadPackage (dist/swipl/swipl.js:9:6290)
      at /home/jesse/Documents/github/npm-swipl-wasm/dist/swipl/swipl.js:9:6308
      at /home/jesse/Documents/github/npm-swipl-wasm/dist/swipl/swipl.js:9:30567
      at module.exports (dist/swipl-node.js:8:10)
      at Context.<anonymous> (tests/node.js:83:30)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

  2) SWI-Prolog WebAssembly on Node.js
       [web] should be able to preload files and generate images:
     TypeError: Module.preRun.push is not a function
      at loadPackage (dist/swipl/swipl-web.js:9:6290)
      at /home/jesse/Documents/github/npm-swipl-wasm/dist/swipl/swipl-web.js:9:6308
      at /home/jesse/Documents/github/npm-swipl-wasm/dist/swipl/swipl-web.js:9:30567
      at Context.<anonymous> (tests/node.js:83:30)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

but this test runs fine on the bundled build.

NOTE These errors come from

swipl-devel/src/wasm/pre.js

Lines 128 to 130 in 4a83822

if ( Module.on_output )
{ Module.preRun.push(bind_std_streams);
}

NOTE: These errors also occur with the original CMAKE file on commit b089616

NOTE: #1103 Resolves this error

jeswr added a commit to jeswr/swipl-devel that referenced this pull request Jan 23, 2023
JanWielemaker pushed a commit that referenced this pull request Jan 23, 2023
JanWielemaker pushed a commit to SWI-Prolog/swipl that referenced this pull request Jan 26, 2023
@jeswr jeswr closed this Jan 30, 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

2 participants