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

Improve huge_patch test #60

Merged
merged 2 commits into from
Jul 13, 2024
Merged

Improve huge_patch test #60

merged 2 commits into from
Jul 13, 2024

Conversation

arkamar
Copy link
Contributor

@arkamar arkamar commented Jun 20, 2024

The huge_patch test can fail in some python interpreters, e.g. pypy3 or python3.13 in my case, because text data preparation can take long time as it could be very memory intensive.

This PR addresses the issue with following two changes:

  1. Move start_time measurement to measure only the parsing time but not data preparation. In other words, test will pass even if the test data preparation will take long time.
  2. The test data are constructed from smaller parts with help of ''.join() method, which is more efficient because it minimizes the number of new string objects created.

The PR is related to #46
See also https://bugs.gentoo.org/907243

We don't want to measure time of test data preparation because it could
be slow in some python interpreters, depending on how string
concatenation is implemented.
Data preparation for huge_patch test could be very slow because strings
are immutable, each concatenation creates a new string and discards
the old ones.

New approach of data preparation is to concatenate large string from
smaller parts with ''.join() method. This method reduces memory usage
and enhances performance, because it minimizes the number of new string
objects created.
gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Jun 20, 2024
The huge_patch test measures performance of the parser together with
test data preparation. It could take long time in some interpreters,
therefore, let's deselect it. This is also related to [1], which
addresses test problems.

[1] cscorley/whatthepatch#60

Closes: https://bugs.gentoo.org/907243
Signed-off-by: Petr Vaněk <[email protected]>
@babenek
Copy link
Contributor

babenek commented Jun 20, 2024

Agree. The test must check parse time.

@cscorley
Copy link
Owner

Looks good to me, thanks!

@cscorley cscorley merged commit 5619626 into cscorley:main Jul 13, 2024
0 of 15 checks passed
@arkamar
Copy link
Contributor Author

arkamar commented Jul 13, 2024

@babenek, @cscorley, thanks!

@arkamar arkamar deleted the huge-patch branch July 13, 2024 20:13
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.

3 participants