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

Fix Recursive Behaviour Lookup Regression #2339

Merged
merged 1 commit into from
Sep 1, 2020

Conversation

ferd
Copy link
Collaborator

@ferd ferd commented Sep 1, 2020

This was due to a subtle mistake in the new code in
#2322 which optimized the search
behaviour of files not found by EPP. The patch mistakenly always kept
the top-level src/ directory for an app even if it did a lookup in
recursive ones.

This patch tracks and maintains the proper subdirectory hierarchy. To
keep performance adequate, the lists:keyfind/3 function is used, which
is also now a NIF to the same extent as lists:member/2; manual lookups
would likely end up reducing performance, particularly in deep
hierarchies.

A test case has been added to track the regression.
Reported by @elbrujohalcon
CC @max-au

This was due to a subtle mistake in the new code in
erlang#2322 which optimized the search
behaviour of files not found by EPP. The patch mistakenly always kept
the top-level src/ directory for an app even if it did a lookup in
recursive ones.

This patch tracks and maintains the proper subdirectory hierarchy. To
keep performance adequate, the lists:keyfind/3 function is used, which
is also now a NIF to the same extent as lists:member/2; manual lookups
would likely end up reducing performance, particularly in deep
hierarchies.

A test case has been added to track the regression.
Reported by @elbrujohalcon
@ferd ferd requested a review from tsloughter September 1, 2020 13:44
@ferd ferd merged commit 7224180 into erlang:master Sep 1, 2020
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