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.

Sign up for GitHub

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

Jump to bottom

Revert #25754 (improvements to localAccess() calls) #25936

Merged
merged 2 commits into from
Sep 12, 2024
Merged

Revert #25754 (improvements to localAccess() calls) #25936

merged 2 commits into from
Sep 12, 2024

Commits on Sep 12, 2024

  1. Revert chapel-lang#25754 

    While chapel-lang#25754 did a nice job of improving the error messages for local
accesses, its implementation was sufficiently heavyweight that it
resulted in new timeouts in testing for tests that made a lot of use
of localAccesses.  On one hand, I didn't anticipate how much overhead
the implementation would take; on the other hand, I incorrectly
thought it would only occur for codes that used localAccess() (with
checks on), forgetting that our auto-local-access (ALA) optimization
would also cause it to fire for codes that didn't.  As a particularly
bad example, Engin found that test/studies/hpcc/PTRANS/old/PTRANS.chpl
went from 35 to 62 seconds on his laptop despite not using any
explicit localAccess calls.

We could just turn off the new checks by default for this release, but
it felt simpler/saner to me to just revert the PR for now.

I think the way to improve this overhead going forward is probably
to change the logic from the current approach of:

* gather all local subdomains a locale owns
  - see if the index we're accessing is within any of them

to:

* ask the distribution itself whether the index is local
* only gather all local subdomains if we get a locality error
  and need to print out what the local locale owns

Even in that case, we could improve on the current logic by doing more
specialization for the (common) "locale only owns a single sublocale"
case than I did in the currenet approach.  However, I think it's fine
to spend more time gathering the subdomains when we're about to print
an error and exit the program anyway.

---
Signed-off-by: Brad Chamberlain <[email protected]>
    @bradcray
    bradcray committed Sep 12, 2024
    Configuration menu
    Copy the full SHA
    d97ce6e View commit details
    Browse the repository at this point in the history

  2. Missed that this chunk failed when reverting my patch 

    ---
Signed-off-by: Brad Chamberlain <[email protected]>
    @bradcray
    bradcray committed Sep 12, 2024
    Configuration menu
    Copy the full SHA
    c79e001 View commit details
    Browse the repository at this point in the history