这是indexloc提供的服务，不要输入任何密码
Skip to content

shallow: stop checking output of base numeric fns #1316

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
Merged
merged 1 commit into from
Mar 5, 2023
Merged

shallow: stop checking output of base numeric fns #1316

merged 1 commit into from
Mar 5, 2023

Conversation

bennn
Copy link
Contributor

@bennn bennn commented Mar 4, 2023 •
edited
Loading

add the default "trust fn result" flag to the definitions at the top of base-env-numeric

thanks @shawnw for the report:
https://www.reddit.com/r/Racket/comments/11dqjbb/experimenting_with_the_new_typed_racket_modes/?utm_source=share&utm_medium=web2x&context=3

@bennn
shallow: stop checking output of base numeric fns
34c52ad 
add the default "trust fn result" flag to the definitions at the top of
base-env-numeric
github-actions[bot]
github-actions bot approved these changes Mar 4, 2023
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resyntax analyzed 1 file in this pull request and found no issues.

@bennn bennn merged commit 700506c into racket:master Mar 5, 2023
@samth
Copy link
Member

samth commented Mar 5, 2023

Does this change any of the benchmarks you've done for shallow?

@shawnw
Copy link

shawnw commented Mar 5, 2023

Does this change any of the benchmarks you've done for shallow?

I don't know; I just use OS packages for racket, never tried building from source to try an in-development version.

@samth
Copy link
Member

samth commented Mar 5, 2023

Sorry, that question was intended for @bennn.

@bennn
Copy link
Contributor Author

bennn commented Mar 9, 2023 •
edited
Loading

Yes! This makes a huge difference for morsecode (many seconds) and +1 seconds on a few others. Really should have caught this earlier. It's time to add Shallow to the perf-monitoring package and run that regularly (gtp-checkup).

Here are quick timings (ms) on the all-Shallow configurations for some of the GTP Benchmarks. Times are the average of 8 runs on a Cloudlab node (Utah m510).

benchmark before ms (stddev) after
morsecode 27925.00 (730.72) 3316.38 (42.28)
suffixtree 10083.75 (360.02) 8652.38 (132.21)
fsm 2879.00 (108.52) 1659.75 (42.32)
tetris 3464.50 (102.95) 2372.00 (27.95)
dungeon 903.13 (23.07) 876.13 (18.72)
gregor 576.13 (11.00) 425.13 (22.41)
synth 883.13 (13.93) 779.00 (25.36)
snake 3794.13 (172.48) 3576.25 (45.23)
kcfa 1385.00 (40.30) 1371.13 (20.55)
lnm 677.88 (4.57) 649.00 (4.30)
zombie 1365.00 (61.64) 1314.13 (19.22)
mbta 2046.25 (39.84) 2013.63 (56.99)
quadT 1469.13 (32.25) 1460.00 (19.32)
quadU 1911.88 (9.53) 1944.13 (22.28)
sieve 4689.75 (89.12) 4048.00 (72.52)

EDIT: These slow numbers for morsecode etc. are relatively new; the bug appeared late in the process of merging Shallow TR. Past benchmark results for Shallow don't show the slowdown (pldi'22, programming'22, my dissertation). That's because past work had simpler way to avoid checks: it kept a list of trusted identifiers. The current approach uses types --- which is more expressive (programming'22 talks about this a bit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bennn @samth @shawnw