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

Prevent calling same class methods inside of schema files #659

Closed
wants to merge 2 commits into from

Conversation

stloyd
Copy link
Member

@stloyd stloyd commented Oct 28, 2023

Change Log

Added

Fixed

Changed

  • Prevent calling same class methods inside of schema files

Removed

Deprecated

Security


Description

Skipping calling additional methods in the matter of accessing internal variables in class improves performance.

See: https://3v4l.org/sOIct

@norberttech
Copy link
Member

Hmm not sure if I get the intention of this, is that affecting performance in any way?

@github-actions
Copy link
Contributor

github-actions bot commented Oct 29, 2023

Flow PHP - Benchmarks

Results of the benchmarks from this PR are compared with the results from 1.x branch.

Extractors

+-----------------------+-------------------+------+-----+-----------------+--------------------+
| benchmark             | subject           | revs | its | mem_peak        | mode               |
+-----------------------+-------------------+------+-----+-----------------+--------------------+
| AvroExtractorBench    | bench_extract_10k | 5    | 3   | 44.121mb        | 517.847ms          |
| CSVExtractorBench     | bench_extract_10k | 5    | 3   | 14.006mb        | 435.235ms          |
| JsonExtractorBench    | bench_extract_10k | 5    | 3   | 18.667mb        | 823.888ms          |
| ParquetExtractorBench | bench_extract_10k | 5    | 3   | 237.791mb       | 1.153s             |
| TextExtractorBench    | bench_extract     | 5    | 3   | 3.935mb +51.22% | 3.189ms +74010.70% |
| XmlExtractorBench     | bench_extract     | 5    | 3   | 3.935mb +21.65% | 1.788ms +49304.36% |
+-----------------------+-------------------+------+-----+-----------------+--------------------+

Transformers

+-------------------------------------+----------------------------+------+-----+----------------+------------------+
| benchmark                           | subject                    | revs | its | mem_peak       | mode             |
+-------------------------------------+----------------------------+------+-----+----------------+------------------+
| EntryExpressionEvalTransformerBench | bench_transform_json_row   | 1000 | 5   | 3.935mb +0.75% | 26.538μs +43.90% |
| EntryExpressionEvalTransformerBench | bench_transform_string_row | 1000 | 5   | 3.935mb +0.75% | 19.546μs +29.90% |
| EntryExpressionEvalTransformerBench | bench_transform_xml_row    | 1000 | 5   | 3.935mb +0.75% | 58.169μs +57.90% |
| RenameEntryTransformerBench         | bench_transform            | 1000 | 5   | 3.935mb +0.76% | 6.324μs -74.35%  |
+-------------------------------------+----------------------------+------+-----+----------------+------------------+

Entry Factory

+-------------------------+----------------+------+-----+----------+-----------+
| benchmark               | subject        | revs | its | mem_peak | mode      |
+-------------------------+----------------+------+-----+----------+-----------+
| NativeEntryFactoryBench | bench_10k_rows | 5    | 5   | 83.901mb | 185.339ms |
| NativeEntryFactoryBench | bench_1k_rows  | 5    | 5   | 45.446mb | 17.297ms  |
| NativeEntryFactoryBench | bench_5k_rows  | 5    | 5   | 62.730mb | 91.050ms  |
+-------------------------+----------------+------+-----+----------+-----------+

@stloyd
Copy link
Member Author

stloyd commented Oct 29, 2023

@norberttech It was late when I pushed this 😂 Added comment into description:

Skipping calling additional methods in the matter of accessing internal variables in class improves performance.
See: 3v4l.org/sOIct

@norberttech
Copy link
Member

I would rather postpone this adjustment for now, there are two reasons:

  • it does not seem to be impactful enough to bring a visible value to the performance at this state
  • For some of those methods I'm planning to make mnemonics that will execute only once (like flatPath), and then return the result like, for example isList or isListElement, that's why I'm using methods directly rather than properties

@stloyd stloyd closed this Oct 29, 2023
@stloyd stloyd deleted the chore/parq-column-internal branch October 29, 2023 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants