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

PathParser.fastPathBuild array allocation seems to do nothing #730

Closed
hkupty opened this issue May 18, 2021 · 5 comments · Fixed by #731
Closed

PathParser.fastPathBuild array allocation seems to do nothing #730

hkupty opened this issue May 18, 2021 · 5 comments · Fixed by #731

Comments

@hkupty
Copy link
Contributor

hkupty commented May 18, 2021

Hi, I was investigating some performance bottlenecks of an application and interestingly this array allocation appears to be on the on the hotpath. On this 5 minute scan, PathParser.fastPathBuild was responsible for ~6.15% of the 1.23GB object array allocation of the system.

When checking, it seems that the array is allocated but never used. seems like its usage was dropped as of this commit, but it was left there. Is that the case?

image

@havocp
Copy link
Collaborator

havocp commented May 18, 2021

Looks true to me! Wouldn't be the first time a thing called "fast" in some code was... not fast. 🙃

@ekrich
Copy link
Contributor

ekrich commented May 18, 2021

A Token is being added to the array but then the Array doesn't seem to be used afterwords.

@viktorklang
Copy link
Contributor

Wow—nice catch!

@hkupty hkupty changed the title PahParser.fastPathBuild array allocation seems to do nothing PathParser.fastPathBuild array allocation seems to do nothing May 18, 2021
@hkupty
Copy link
Contributor Author

hkupty commented May 18, 2021

Nice.. I can open a PR to fix - essentially removing those two lines above. Not sure if that's the intended behavior of this function though.

hkupty added a commit to hkupty/config that referenced this issue May 18, 2021
Usage of this array seems to have been dropped by lightbend#280, but the array
remained.

Cleaning up so it doesn't allocate the array needlessly anymore.

Fixes lightbend#730.
@ekrich
Copy link
Contributor

ekrich commented May 18, 2021

Nice catch. I removed it from sconfig as well.

hkupty added a commit to hkupty/config that referenced this issue May 21, 2021
Usage of this array seems to have been dropped by lightbend#280, but the array
remained.

Cleaning up so it doesn't allocate the array needlessly anymore.

Fixes lightbend#730.
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 a pull request may close this issue.

4 participants