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

Atomic keys introduction for strings, symbols, small numbers. #848

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

VadimZhestikov
Copy link
Contributor

@VadimZhestikov VadimZhestikov commented Feb 6, 2025

Proposed changes

This is series of commits which introduce internal atomic representation for
strings, symbols and small numbers as keys for access object properties.

These commits are intended to speed up access to object properties.

Checklist

Before creating a PR, run through this checklist and mark each as complete:

  • I have read the CONTRIBUTING document
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes

@VadimZhestikov VadimZhestikov changed the title Atomic strings new Atomic keys introduction for strings, symbols, small numbers. Feb 6, 2025
@VadimZhestikov
Copy link
Contributor Author

VadimZhestikov commented Feb 8, 2025

The issue is obtained by check-pr, when dynamic modules are configured with quickjs-ng, for case when 'js_engine qjs;' is not used. I still trying reproduce it on my local pc, but those tests run w/o issues for me. Continuing.

@VadimZhestikov VadimZhestikov force-pushed the atomic_strings_new branch 13 times, most recently from 269870d to d0dc432 Compare February 14, 2025 22:46
-- use njs_atom_atomize_key();
-- remove symbol_generator;
-- remove NJS_SYMBOL_KNOWN_MAX;
-- use atom_ids in njs_object_hash_test();
-- use atom_id as key_hash;
-- remove NJS_*_HASH;
-- remove not used files;
-- remove name_value except for atom_id from
   njs_object_prop_s.

Performance (bench4.js) for reference:

   before all atomic patches:
      Richards: 386
      Crypto: 933
      Splay: 1752
      NavierStokes: 1336
      ----
      Score (version 7): 959

   after this patch:
      Richards: 541
      Crypto: 931
      Splay: 2039
      NavierStokes: 1224
      ----
      Score (version 7): 1059
Performance (bench4.js) for reference:

   after this patch:
      Richards: 561
      Crypto: 1030
      Splay: 2078
      NavierStokes: 1225
      ----
      Score (version 7): 1101
Performance (bench4.js) for reference:

   after this patch:
      Richards: 686
      Crypto: 1028
      Splay: 2023
      NavierStokes: 1652
      ----
      Score (version 7): 1239
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.

1 participant