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

Major source code optimization #1785

Merged
merged 12 commits into from Jan 10, 2014
Merged

Major source code optimization #1785

merged 12 commits into from Jan 10, 2014

Conversation

ghost
Copy link

@ghost ghost commented Jan 10, 2014

In brief, what has been done:

  • all header files had include guards added
  • arginfo and class method tables were moved from headers to sources (exception: interfaces and bases classes have arginfo in the header — this allows to reuse it by the child classes and interface implementors)
  • source files do not #include "phalcon.h" anymore — they #include only the files they need
  • zend_class_entry definitions were moved from phalcon.c to respective source files — now if someone needs to add a new class, there are less places in the code to modify
  • gcc-compatible compilers are instructed to generate dependency files (*.d): this is what automake usually does but because PHP uses only autoconf, this had to be done manually (pros: if you change a file, all dependent files need to be rebuilt. Before this, if you touch a header file, make won't rebuild anything and you need to rebuild the entire project).

Why this is good:

  • the compiler has to process fewer files (when the source file #include'd phalcon.h, the compiler had to process all headers regardless whether the source file needed them)
  • include guards will allow to use static inline functions in the headers — this could be used to replace macros (functions are type safe, macros aren't)
  • less symbols are exposed, this reduces the size of the debug information and allows for extra optimizations.

Benchmarks:

1.3.0 branch

$ du -hs ext 
7,5M    ext

$ cd ext; time sh -c 'phpize --clean && phpize && ./configure CFLAGS="-O1 -g -Wall -Werror" && make -B -j75 install'

real    1m37.913s
user    3m17.824s
sys     0m46.596s

Max LA: 55.12

$ du -h modules/phalcon.so

193M    modules/phalcon.so

$ NO_INTERACTION=1 make test

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :    0
Exts tested     :   46
---------------------------------------------------------------------

Number of tests :   91                85
Tests skipped   :    6 (  6.6%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   85 ( 93.4%) (100.0%)
---------------------------------------------------------------------
Time taken      :   28 seconds
=====================================================================

This branch:

$ du -hs ext 
7,6M    ext

$ cd ext; time sh -c 'phpize --clean && phpize && ./configure CFLAGS="-O1 -g -Wall -Werror" && make -B -j75 install'

real    0m41.058s
user    1m13.126s
sys     0m37.876s

Max LA: 30.77

$ du -h modules/phalcon.so
9,6M    modules/phalcon.so

$ NO_INTERACTION=1 make test

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :    0
Exts tested     :   46
---------------------------------------------------------------------

Number of tests :   91                85
Tests skipped   :    6 (  6.6%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   85 ( 93.4%) (100.0%)
---------------------------------------------------------------------
Time taken      :   10 seconds
=====================================================================

Summary:

  • 1.3.0:
    • compilation time: 1m37.913s
    • max load average: 55.12
    • binary size: 193M
    • time taken for tests: 28 seconds
  • this branch:
    • compilation time: 41.058s (58.07% faster)
    • max load average: 30.77 (44.18% less)
    • binary size: 9.6M (95.03% smaller)
    • time taken for tests: 10 seconds (64% faster)

@ghost
Copy link
Author

ghost commented Jan 10, 2014

The other thing I like is this: 722 changed files with 63,212 additions and 85,570 deletions

In other words, this reorganization made the source code smaller.

phalcon pushed a commit that referenced this pull request Jan 10, 2014
Major source code optimization
@phalcon phalcon merged commit 771d909 into phalcon:1.3.0 Jan 10, 2014
@phalcon
Copy link
Collaborator

phalcon commented Jan 10, 2014

Amazing!!!

@ghost ghost deleted the deps branch January 10, 2014 23:38
@ghost
Copy link
Author

ghost commented Jan 10, 2014

Yeah, I have been working on that since October or so :-)

@phalcon
Copy link
Collaborator

phalcon commented Jan 10, 2014

Great work 👍

@pgasiorowski
Copy link
Contributor

Nice work! Cannot wait to build it

@ovr
Copy link
Contributor

ovr commented Jan 11, 2014

@sjinks Уникальный человек) С твоими мозгами и любовью к программированию тебе уже пора работать в Google!)

@xboston
Copy link
Contributor

xboston commented Jan 11, 2014

Awesome!

@temuri416
Copy link
Contributor

Chapeau!!!

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

7 participants