Skip to content

Optimize some std mem methods by replacing expensive rem operations to fast bit logic #24

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

Merged
merged 2 commits into from
Feb 12, 2018
Merged

Conversation

MaxGraey
Copy link
Member

@MaxGraey MaxGraey commented Feb 8, 2018

No description provided.

@MaxGraey MaxGraey changed the title Optimize some std mem operation by replacing expensive rem operatins to fast bit logic Optimize some std mem methods by replacing expensive rem operations to fast bit logic Feb 8, 2018
@dcodeIO
Copy link
Member

dcodeIO commented Feb 8, 2018

Seems like a good issue for Binaryen's optimizer as well.

@MaxGraey
Copy link
Member Author

MaxGraey commented Feb 8, 2018

Yep, I thought about it as well

@MaxGraey
Copy link
Member Author

MaxGraey commented Feb 8, 2018

And what you think if replace this:

    store<u32>(dest     , load<u32>(src     ));
    store<u32>(dest +  4, load<u32>(src +  4));
    store<u32>(dest +  8, load<u32>(src +  8));
    store<u32>(dest + 12, load<u32>(src + 12));
    src += 16; dest += 16; n -= 16;
    src += 16; dest += 16; n -= 16;

to:

    store<u64>(dest     , load<u64>(src     ));
    store<u64>(dest +  8, load<u64>(src +  8));
    src += 16; dest += 16; n -= 16;
    src += 16; dest += 16; n -= 16;

for aligned memory copy?

@dcodeIO
Copy link
Member

dcodeIO commented Feb 8, 2018

Depends whether dest and src are guaranteed to be aligned to 8 bytes at this point.

@dcodeIO
Copy link
Member

dcodeIO commented Feb 8, 2018

Seems not, as it starts with if (dest % 4 == 0)

@MaxGraey
Copy link
Member Author

MaxGraey commented Feb 8, 2018

You are right. But for classes and arrays we can expecting 8 byte alignment when mem layout will be finish of course, right? So It's exactly for that case (dest % 8 == 0) and It seems this additional part, not replacment

@dcodeIO
Copy link
Member

dcodeIO commented Feb 8, 2018

Well, imagine an Uint8Array. The class would be aligned to 4 bytes because it starts with an i32 length (unless WASM64 is the target, requireing a pointer to the array being aligned to 8 bytes). Its data, which is a separate block, would be aligned to 4 bytes just because that's the minimum alignment (otherwise it'd be 1).

Don't worry too much about these. The copy_memory and move_memory stuff will be replaced by bulk memory operations anyway, once these have landed.

@MaxGraey
Copy link
Member Author

MaxGraey commented Feb 8, 2018

Ok, I got it

@dcodeIO
Copy link
Member

dcodeIO commented Feb 12, 2018

These changes are looking good to me. Please include yourself in the NOTICE file before I merge :)

@MaxGraey
Copy link
Member Author

MaxGraey commented Feb 12, 2018

Done! Thanks;)

@dcodeIO dcodeIO merged commit 2175e6f into AssemblyScript:master Feb 12, 2018
@dcodeIO
Copy link
Member

dcodeIO commented Feb 12, 2018

Thanks :)

@MaxGraey MaxGraey deleted the mem-improvments branch May 25, 2019 13:16
willemneal pushed a commit to willemneal/assemblyscript that referenced this pull request May 31, 2019
radu-matei pushed a commit to radu-matei/assemblyscript that referenced this pull request Oct 13, 2020
radu-matei pushed a commit to radu-matei/assemblyscript that referenced this pull request Oct 13, 2020
* 'master' of github.com:jedisct1/wasa:
  Fixed the memory leak for Time.sleep (AssemblyScript#33)
  Improve Descriptor (AssemblyScript#25)
  Bump assemblyscript from 0.9.2 to 0.9.4 (AssemblyScript#27)
  Deps
  Optimizations + update dependencies (AssemblyScript#21)
  Bump @as-pect/cli from 2.7.0 to 2.8.1 (AssemblyScript#23)
  Bump assemblyscript from 0.9.1 to 0.9.2 (AssemblyScript#24)
  Fix Date.now to return milliseconds instead of microseconds (AssemblyScript#19)
  Bump assemblyscript from 0.9.0 to 0.9.1 (AssemblyScript#17)
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.

2 participants