Skip to content

Remove allocation in parse_identifier#12103

Merged
MichaReiser merged 1 commit intomainfrom
parse-identifier-reduce-alloc
Jun 29, 2024
Merged

Remove allocation in parse_identifier#12103
MichaReiser merged 1 commit intomainfrom
parse-identifier-reduce-alloc

Conversation

@MichaReiser
Copy link
Copy Markdown
Member

@MichaReiser MichaReiser commented Jun 29, 2024

Summary

I think this PR removes an allocation from parse_identifier.

parse_identifier converts the Box<str> from the TokenValue::Name to a String by calling .to_string.
to_string is a small wrapper around format!("{name}") which allocates a new string without reusing the Box<str> allocation, unless the compiler can see through the operation.

This PR uses Box<str>::into_string which, to my knowledge, reuses the underlying allocation.

Test Plan

cargo test

@MichaReiser MichaReiser requested a review from dhruvmanila as a code owner June 29, 2024 12:55
@MichaReiser MichaReiser added performance Potential performance improvement parser Related to the parser labels Jun 29, 2024
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Jun 29, 2024

CodSpeed Performance Report

Merging #12103 will improve performances by 7.44%

Comparing parse-identifier-reduce-alloc (83894c9) with main (47b2273)

Summary

⚡ 4 improvements
✅ 26 untouched benchmarks

Benchmarks breakdown

Benchmark main parse-identifier-reduce-alloc Change
⚡ parser[large/dataset.py] 5.4 ms 5 ms +7.44%
⚡ parser[numpy/ctypeslib.py] 1,007.9 µs 954.6 µs +5.59%
⚡ parser[pydantic/types.py] 2.2 ms 2.1 ms +5.9%
⚡ parser[unicode/pypinyin.py] 329.3 µs 315 µs +4.54%

@MichaReiser
Copy link
Copy Markdown
Member Author

MichaReiser commented Jun 29, 2024

I'll go ahead and merge this. This change should be pretty uncontroversial 😆

@MichaReiser MichaReiser merged commit da78de0 into main Jun 29, 2024
@MichaReiser MichaReiser deleted the parse-identifier-reduce-alloc branch June 29, 2024 13:00
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 29, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check encountered linter errors. (no lint changes; 1 project error)

demisto/content (error) 

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'ignore' -> 'lint.ignore'
  - 'select' -> 'lint.select'
  - 'unfixable' -> 'lint.unfixable'
  - 'per-file-ignores' -> 'lint.per-file-ignores'
warning: `PGH001` has been remapped to `S307`.
warning: `PGH002` has been remapped to `G010`.
warning: `PLR1701` has been remapped to `SIM101`.
ruff failed
  Cause: Selection of deprecated rule `E999` is not allowed when preview is enabled.

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error) 

warning: Detected debug build without --no-cache.
error: Failed to parse examples/gpt_actions_library/.gpt_action_getting_started.ipynb:11:1:1: Expected an expression
error: Failed to parse examples/gpt_actions_library/gpt_action_bigquery.ipynb:13:1:1: Expected an expression

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 2 project errors)

demisto/content (error) 

ruff format --preview --exclude Packs/ThreatQ/Integrations/ThreatQ/ThreatQ.py

warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'ignore' -> 'lint.ignore'
  - 'select' -> 'lint.select'
  - 'unfixable' -> 'lint.unfixable'
  - 'per-file-ignores' -> 'lint.per-file-ignores'
warning: `PGH001` has been remapped to `S307`.
warning: `PGH002` has been remapped to `G010`.
warning: `PLR1701` has been remapped to `SIM101`.
ruff failed
  Cause: Selection of deprecated rule `E999` is not allowed when preview is enabled.

openai/openai-cookbook (error) 

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to parse examples/gpt_actions_library/.gpt_action_getting_started.ipynb:11:1:1: Expected an expression
error: Failed to parse examples/gpt_actions_library/gpt_action_bigquery.ipynb:13:1:1: Expected an expression

@MichaReiser MichaReiser mentioned this pull request Jun 29, 2024
Merged
@AlexWaygood AlexWaygood changed the title Remove allcation in parse_identifier Remove allocation in parse_identifier Jun 29, 2024
@BrewTestBot BrewTestBot mentioned this pull request Jul 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dhruvmanila dhruvmanila Awaiting requested review from dhruvmanila dhruvmanila is a code owner

Assignees

No one assigned

Labels

parser Related to the parser performance Potential performance improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@MichaReiser