Skip to content

dart/entrypoints/jit tests are overly sensitive to usage counters #37144

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
Closed
rmacnak-google opened this issue Jun 3, 2019 · 3 comments
Closed

dart/entrypoints/jit tests are overly sensitive to usage counters #37144

rmacnak-google opened this issue Jun 3, 2019 · 3 comments
Assignees
@sjindel-google
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test).

Comments

@rmacnak-google
Copy link
Contributor

These tests are disabled in https://dart-review.googlesource.com/c/sdk/+/102820 because of changes in the way the usage counter is incremented.
@rmacnak-google rmacnak-google added the area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). label Jun 3, 2019
@mkustermann
Copy link
Member

The CL which is up for review wants to dart/entrypoints/jit: Skip. This would make our checked/unchecked entrypoint functionality untested. It was introduced to bypass Dart 2.0 type checks on function entry if it's safe, which gave - as far as I've been told - significant performance improvements.

Can you elaborate why exactly they will be skipped and if there's a plan to enable them again?

dart-bot pushed a commit that referenced this issue Jul 18, 2019
@alexmarkov
Revert "[vm/tests] Re-enable tests for multiple entry points"
38deece 
This reverts commit 00eb034.

Reason for revert: test failure on vm-kernel-optcounter-threshold-linux-release-x64 bot

Original change's description:
> [vm/tests] Re-enable tests for multiple entry points
> 
> Closes #37144
> 
> Change-Id: I9935db44cf01dda106c9211978dca33aac7fcb16
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109551
> Reviewed-by: Ryan Macnak <[email protected]>
> Commit-Queue: Alexander Markov <[email protected]>

[email protected],[email protected],[email protected]

Change-Id: I582c014c4e02216be1575609aa950e8afb7e0702
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109600
Reviewed-by: Alexander Markov <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>
dart-bot pushed a commit that referenced this issue Jul 19, 2019
@alexmarkov
[vm/tests] Re-enable tests for multiple entry points, take 2.
09418f4 
Closes #37144

This is a re-land of 00eb034.

Change-Id: Ib46d4590de460a520415280b3253e75d8f947b81
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109584
Reviewed-by: Ryan Macnak <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>
@sjindel-google
Copy link
Contributor

I think we should just emit the unchecked calls in optimized and unoptimized code. I can't see a reason to not do it in unoptimized code.

This would make the tests completely insensitive to usage counters.

@sjindel-google sjindel-google self-assigned this Jul 19, 2019
@sjindel-google
Copy link
Contributor

It turns out this requires updating all the IC stubs

tekknolagi pushed a commit to tekknolagi/dart-assembler that referenced this issue Nov 3, 2020
@sjindel-google
[vm] Enable multiple entry-points for unoptimized calls.
2e3706e 
This also makes the entrypoints tests less sensitive to the order in which
functions are optimized and removes flaky checks.

Fixes dart-lang#37144.
Fixes dart-lang#39447.

Change-Id: I5ba23c74769ddb8415e2b635caefc798c2b70500
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109704
Commit-Queue: Samir Jindel <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sjindel-google sjindel-google

Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test).
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mkustermann @rmacnak-google @sjindel-google