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

Jump to bottom

optimize some code paths #361

Closed
wants to merge 2 commits into from
Closed

optimize some code paths #361

wants to merge 2 commits into from

Conversation

atetubou
Copy link
Contributor

This optimizes mako template engine by reducing the number of regex compiles and replace some regex usage with simpler string utility function.

This shows around 10% performance improvement in our use case (https://crbug.com/1214033#c32).

@zzzeek
Copy link
Member

zzzeek commented Jun 29, 2022

OK can you run "black" on the code changes so it passes pep8, thanks!

@zzzeek zzzeek requested a review from sqla-tester June 29, 2022 12:44
sqla-tester
sqla-tester reviewed Jun 29, 2022
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 6c1b2ee of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

New Gerrit review created for change 6c1b2ee: https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/3959

@atetubou
Copy link
Contributor Author

I applied black, could you run check again?

@zzzeek zzzeek requested a review from sqla-tester June 30, 2022 13:25
sqla-tester
sqla-tester reviewed Jun 30, 2022
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision bcdee5c of this pull request into gerrit so we can run tests and reviews and stuff

@zzzeek zzzeek requested a review from sqla-tester June 30, 2022 14:40
sqla-tester
sqla-tester reviewed Jun 30, 2022
View reviewed changes
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision bcdee5c of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

Patchset bcdee5c added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/3959

@sqla-tester
Copy link
Collaborator

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/3959 has been merged. Congratulations! :)

@atetubou atetubou deleted the optimize_mako branch July 1, 2022 00:27
@atetubou
Copy link
Contributor Author

atetubou commented Jul 1, 2022

Thank you for merge and quick release!

aarongable pushed a commit to chromium/chromium that referenced this pull request Jul 1, 2022
mako: update version to 1.2.1 from 1.2.0
c4edcb4 
This is to import sqlalchemy/mako#361.

This update shows 14% improvement in walltime and uses only 80% of cpu
time compared to current HEAD.

```
$ hyperfine --parameter-list branch origin/main,mako_optimize --setup 'git checkout {branch}' 'python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union'
Benchmark 1: python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union
  Time (mean ± σ):     33.499 s ±  0.289 s    [User: 930.572 s, System: 20.354 s]
  Range (min … max):   32.971 s … 33.915 s    10 runs

Benchmark 2: python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union
  Time (mean ± σ):     29.261 s ±  0.298 s    [User: 737.851 s, System: 20.448 s]
  Range (min … max):   28.737 s … 29.835 s    10 runs

Summary
  'python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union' ran
    1.14 ± 0.02 times faster than 'python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union'
```


Bug: 1214033
Change-Id: I54eab0e27872d40256d3ecb04c47c876652b5596
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3685551
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1019918}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
mako: update version to 1.2.1 from 1.2.0
915cacc 
This is to import sqlalchemy/mako#361.

This update shows 14% improvement in walltime and uses only 80% of cpu
time compared to current HEAD.

```
$ hyperfine --parameter-list branch origin/main,mako_optimize --setup 'git checkout {branch}' 'python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union'
Benchmark 1: python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union
  Time (mean ± σ):     33.499 s ±  0.289 s    [User: 930.572 s, System: 20.354 s]
  Range (min … max):   32.971 s … 33.915 s    10 runs

Benchmark 2: python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union
  Time (mean ± σ):     29.261 s ±  0.298 s    [User: 737.851 s, System: 20.448 s]
  Range (min … max):   28.737 s … 29.835 s    10 runs

Summary
  'python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union' ran
    1.14 ± 0.02 times faster than 'python3 ../../third_party/blink/renderer/bindings/scripts/generate_bindings.py --web_idl_database gen/third_party/blink/renderer/bindings/web_idl_database.pickle --root_src_dir ../../ --root_gen_dir gen --output_reldir core=third_party/blink/renderer/bindings/core/v8/ --output_reldir modules=third_party/blink/renderer/bindings/modules/v8/ enumeration callback_function callback_interface dictionary interface namespace observable_array typedef union'
```

Bug: 1214033
Change-Id: I54eab0e27872d40256d3ecb04c47c876652b5596
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3685551
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1019918}
NOKEYCHECK=True
GitOrigin-RevId: c4edcb406fe832cad04012fcbe000b8c09a836a2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sqla-tester sqla-tester sqla-tester left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@atetubou @zzzeek @sqla-tester