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

current status of AOT_CURRENT_VERSION #3837

Closed
Zzzabiyaka opened this issue Oct 4, 2024 · 19 comments · Fixed by #3880
Closed

current status of AOT_CURRENT_VERSION #3837

Zzzabiyaka opened this issue Oct 4, 2024 · 19 comments · Fixed by #3880

Comments

@Zzzabiyaka
Copy link
Contributor

Zzzabiyaka commented Oct 4, 2024

Hi!

I see in the docs that upon breaking AOT ABI we bump AOT_CURRENT_VERSION
https://github.com/bytecodealliance/wasm-micro-runtime/blame/0e05b0a451008c4ae95527613537ab02e6f86e5e/doc/build_wasm_app.md#L377

Looking at git blame I am not sure we did that when we introduced version 2.x.y (which I suppose breaks the ABI for AOT)
https://github.com/bytecodealliance/wasm-micro-runtime/blame/0e05b0a451008c4ae95527613537ab02e6f86e5e/core/config.h#L87

Is there still an intent to keep this flag reflecting the "AOT ABI" version or presumably we always will be bumping major version of WAMR instead when we break AOT ABI in the future ?

@wenyongh
Copy link
Contributor

wenyongh commented Oct 8, 2024

@Zzzabiyaka please refer to doc/semantic_version.md and WAMR-2.0.0 release notes, the last time we upgraded AOT_CURRENT_VERSION was due to the merging of new feature GC and memory64. Normally we will try our best to keep the backward compatibility of AOT ABI and not to upgrade AOT_CURRENT_VERSION, but if it is really hard to maintain (e.g. new important feature breaks the ABI), we will upgrade it.

@yamt
Copy link
Collaborator

yamt commented Oct 24, 2024

the last time we upgraded AOT_CURRENT_VERSION was due to the merging of new feature GC and memory64.

actually the last time we bumped it was for reference-types. #612
it was in 2021.

we didn't bump it for GC or memory64.

i guess we have to admit we often forget to bump AOT_CURRENT_VERSION. :-)

@lum1n0us
Copy link
Collaborator

actually the last time we bumped it was for reference-types. #612
it was in 2021.
we didn't bump it for GC or memory64.

@TianlongLiang @wenyongh next release? or make a 2.2.1?

@TianlongLiang
Copy link
Contributor

Yeah, I think it's a good idea to bump it in the next release since GC and memory64 changed the ABI.

yamt added a commit to yamt/wasm-micro-runtime that referenced this issue Oct 24, 2024
Maybe it's too late because we have already made a few releases
since then.
But this might still help users who haven't upgraded to WAMR 2.x yet.
Also, for the purpose of the versioning, it's safer to bump
needlessly than missing necessary bumps.

Fixes bytecodealliance#3837
@wenyongh
Copy link
Contributor

actually the last time we bumped it was for reference-types. #612
it was in 2021.
we didn't bump it for GC or memory64.

@TianlongLiang @wenyongh next release? or make a 2.2.1?

If to bump the AOT_CURRENT_VERSION from 3 to 4, I guess it is better to release WAMR-3.0.0 next time since the old AOT file won't be able to run in the new runtime. But the issue is that will we create a branch dev/release_2.2.x and maintain it?
@yamt @lum1n0us how do you think?

@yamt
Copy link
Collaborator

yamt commented Oct 24, 2024

actually the last time we bumped it was for reference-types. #612
it was in 2021.
we didn't bump it for GC or memory64.

@TianlongLiang @wenyongh next release? or make a 2.2.1?

If to bump the AOT_CURRENT_VERSION from 3 to 4, I guess it is better to release WAMR-3.0.0 next time since the old AOT file won't be able to run in the new runtime. But the issue is that will we create a branch dev/release_2.2.x and maintain it? @yamt @lum1n0us how do you think?

the major version implies many things. not only aot abi.
i suspect that, for majority of users, the api compatibility (eg. wasm_export.h) is far more important than aot abi.
i'm afraid that bumping to 3.0.0 can disturb more users than necessary.

@Zzzabiyaka
Copy link
Contributor Author

i guess we have to admit we often forget to bump AOT_CURRENT_VERSION. :-)

On that topic, I am still wondering whether it makes sense to just stick to what's in doc/semantic_version.md

By doing that, we could effectively deprecate AOT_CURRENT_VERSION in favour of just major version of WAMR
What do you think?

@wenyongh
Copy link
Contributor

actually the last time we bumped it was for reference-types. #612
it was in 2021.
we didn't bump it for GC or memory64.

@TianlongLiang @wenyongh next release? or make a 2.2.1?

If to bump the AOT_CURRENT_VERSION from 3 to 4, I guess it is better to release WAMR-3.0.0 next time since the old AOT file won't be able to run in the new runtime. But the issue is that will we create a branch dev/release_2.2.x and maintain it? @yamt @lum1n0us how do you think?

the major version implies many things. not only aot abi. i suspect that, for majority of users, the api compatibility (eg. wasm_export.h) is far more important than aot abi. i'm afraid that bumping to 3.0.0 can disturb more users than necessary.

OK, but we had better make the explanation to users why bump AOT version to 4, I am not sure whether adding statement in the release notes is enough.

@wenyongh
Copy link
Contributor

i guess we have to admit we often forget to bump AOT_CURRENT_VERSION. :-)

On that topic, I am still wondering whether it makes sense to just stick to what's in doc/semantic_version.md

By doing that, we could effectively deprecate AOT_CURRENT_VERSION in favour of just major version of WAMR What do you think?

It is an option but it really makes confusing, I am not very sure whether it is good, #3880 even bumps AOT_CURRENT_VERSION from 3 to 4, while the current major version is 2, so the gap is bigger. But it seems good as well as we can bump the aot version and major version together in the future, e.g. aot version is 5 and major version is 3.

@lum1n0us
Copy link
Collaborator

maintain a table into doc?

WAMR_RELEASE_VERSION AOT_VERSION features
... 2 xxx
... 3 +yyy
... 4 +memory64,+gc

since the old AOT file won't be able to run in the new runtime.

🤔 ☁️ Wondering that could we support both version 3 and version 4 instead of dropping version 3? We could inform users that features like MEMORY64 and GC are linked with version 4. With version 3, only the older features would be available.

@wenyongh
Copy link
Contributor

wenyongh commented Oct 25, 2024

In fact there are two kinds of version 3: version 3 without GC+MEMORY64 and version 3 with GC+MEMORY64, the former is supported in branch release/1.3.x and we had claimed that we will maintain it until the end of this year. The latter is supported since WAMR-2.0.0 and is used in the current main branch, and WAMR-2.0.0 and main branch don't support the older version 3 again. So there is no obvious difference in the AOT file format except version number if we bump 3 to 4, I am afraid whether it is necessary to bump, we missed to bump it before, but it seems it doesn't harm for the main branch? Seems no one will run the older AOT file in the runtime compiled from main branch, is it better to let it be?

@lum1n0us
Copy link
Collaborator

So there is no obvious difference in the AOT file format exception version number if we bump 3 to 4, I am afraid whether it is necessary to bump.

Agreed. If there's no clear risk of compatibility issues, there's no need to bump the version number.

@yamt
Copy link
Collaborator

yamt commented Oct 25, 2024

a data point: a user upgrading from 1.x to 2.x was confused by the same AOT_CURRENT_VERSION very recently. (a few days ago)

i guess he is not the last user to upgrade from 1.x. bumping to 4 now still can help such users.

@lum1n0us
Copy link
Collaborator

IIUC, we can either match AOT_VERSION with WAMR_VERSION and raise both at every release, or we treat them as two distinct series and bump each one according to their own updates or major changes. In the end, we need to choose one policy and stick to it.

@wenyongh
Copy link
Contributor

I discussed with @lum1n0us and agreed on some suggestions:

  1. bump AOT_CURRENT_VERSION from 3 to 4
  2. modify aot loader to support both AOT_CURRENT_VERSION 3 and 4, just treat them as the same, since there is no difference from 3 and 4 in the main branch, and many developers are using 3 of the main branch, we had better support it
  3. upgrade AOT_CURRENT_VERSION and WAMR major version together in the future, e.g., AOT_CURRENT_VERSION 5 and wamr major version 3, or 6 and 4
  4. maintain a branch release/2.n.x when we upgrade AOT_CURRENT_VERSION to 5 and WAMR major version to 3 in the feature

@yamt, @Zzzabiyaka how do you think? thanks.

@yamt
Copy link
Collaborator

yamt commented Oct 28, 2024

I discussed with @lum1n0us and agreed on some suggestions:

1. bump AOT_CURRENT_VERSION from 3 to 4

2. modify aot loader to support both AOT_CURRENT_VERSION 3 and 4, just treat them as the same, since there is no difference from 3 and 4 in the main branch, and many developers are using 3 of the main branch, we had better support it

3. upgrade AOT_CURRENT_VERSION and WAMR major version together in the future, e.g., AOT_CURRENT_VERSION 5 and wamr major version 3, or 6 and 4

4. maintain a branch `release/2.n.x` when we upgrade AOT_CURRENT_VERSION to 5 and WAMR major version to 3 in the feature

@yamt, @Zzzabiyaka how do you think? thanks.

sounds reasonable to me.

@Zzzabiyaka
Copy link
Contributor Author

I discussed with @lum1n0us and agreed on some suggestions:

  1. bump AOT_CURRENT_VERSION from 3 to 4

  2. modify aot loader to support both AOT_CURRENT_VERSION 3 and 4, just treat them as the same, since there is no difference from 3 and 4 in the main branch, and many developers are using 3 of the main branch, we had better support it

  3. upgrade AOT_CURRENT_VERSION and WAMR major version together in the future, e.g., AOT_CURRENT_VERSION 5 and wamr major version 3, or 6 and 4

  4. maintain a branch release/2.n.x when we upgrade AOT_CURRENT_VERSION to 5 and WAMR major version to 3 in the feature

@yamt, @Zzzabiyaka how do you think? thanks.

Lgtm

@lum1n0us
Copy link
Collaborator

#3891

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 a pull request may close this issue.

5 participants