-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ext/bcmath: Avoid unnecessary memset
#14180
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
ext/bcmath: Avoid unnecessary memset
#14180
Conversation
Intuitively, this speed gain feels strange because memset only takes up 0.35% of runtime on my machine for 1.php for example. Furthermore, I actually see a slowdown when applying this patch on current master:
I see a similar slowdown for your old benchmark script with microtime. |
Thanks. It seems the measurements on my machine are unreliable.... I will try to prepare a stable measurement environment such as EC2. |
@nielsdos master:
this branch:
|
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.
Okay, fine by me then.
Something influences my results and I'm not sure what. Might be caches or micro-architectural stuff.
Is the compiler gcc? I am compiling both with gcc on ubuntu. It might be worth checking if a different assembly is being generated. |
I'm using gcc 13 |
My machine is 9, but the EC2 is 13. Could you please send me the assembler code later when you have time? I'd like to compare it with mine. |
This is the assembly for this PR's doaddsub.s: https://gist.github.com/nielsdos/551b59b07a25df46b8416edc31f89a73 However, I'm not so sure the problem is the assembly, I think it's some CPU magic (e.g. microarchitecture or cache state or something like that) that is coincidentally influenced with this change. |
Apply the same changes as #14180 to _bc_do_add.
0 padding on the right is not required for calc. This is because when converting back to string,
zend_string
is padded with0
.php-src/ext/bcmath/libbcmath/src/num2str.c
Line 68 in 0642855
(I will follow up for add later.)
benchmark: #14132
before
after