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

fix: patterns impose resource limits #6057

Merged
merged 5 commits into from
Sep 7, 2022
Merged

fix: patterns impose resource limits #6057

merged 5 commits into from
Sep 7, 2022

Conversation

erights
Copy link
Member

@erights erights commented Aug 26, 2022 •
edited
Loading

For various type-oriented patterns like M.array(), where the pattern itself does not naturally imply a length limit,

  • add an optional limits argument to the pattern, where the limits argument is a record of optional named limits
  • provide generous default limits that applies if no explicit limits are provided.

The limits are expressed as numbers, not bigints. Unlimited size can thus still be expressed with a limit value of Infinity.

The limits are
* decimalDigitsLimit, for M.bigint, M.nat, and the cardinality of M.bag and M.bagOf
* stringLengthLimit, for M.string
* symbolNameLengthLimit, for M.symbol
* numPropertiesLimit, for M.record and M.recordOf
* propertyNameLengthLimit, for M.record and M.recordOf
* arrayLengthLimit, for M.array and M.arrayOf
* numSetElementsLimit, for M.set and M.setOf
* numUniqueBagElementsLimit:, for M.bag and M.bagOf
* numMapEntriesLimit, for M.map and M.mapOf

See
#5955 (fixes it?)
#5956
#5954
#6098
#6060
#6061
#6062
#5956

@erights erights self-assigned this Aug 26, 2022
@erights erights force-pushed the markm-size-limits-take2 branch from 3363055 to 4140eb3 Compare August 26, 2022 08:55
@erights erights mentioned this pull request Aug 26, 2022
Closed
@erights erights force-pushed the markm-far-classes branch from 3120559 to 3d1a71a Compare August 26, 2022 20:00
@erights erights force-pushed the markm-size-limits-take2 branch from fd3ca19 to e0ff2a1 Compare August 26, 2022 20:01
@erights erights force-pushed the markm-far-classes branch 2 times, most recently from 1e96f9b to e582783 Compare August 27, 2022 03:12
@erights erights force-pushed the markm-size-limits-take2 branch from e0ff2a1 to 92a155b Compare August 27, 2022 03:14
@erights erights force-pushed the markm-far-classes branch from e582783 to 8e2df6b Compare August 27, 2022 17:57
@erights erights force-pushed the markm-size-limits-take2 branch from f045839 to a55f1c6 Compare August 27, 2022 17:59
@erights erights force-pushed the markm-far-classes branch from 8e2df6b to 060b06c Compare August 27, 2022 18:04
@erights erights force-pushed the markm-size-limits-take2 branch from a55f1c6 to b089acc Compare August 27, 2022 18:05
@erights erights force-pushed the markm-far-classes branch from 060b06c to 2dafd4d Compare August 27, 2022 20:10
@erights erights force-pushed the markm-size-limits-take2 branch from ba76966 to a3aa179 Compare August 27, 2022 20:11
@erights erights force-pushed the markm-far-classes branch from 2dafd4d to fcc3bb4 Compare August 28, 2022 19:30
@erights erights force-pushed the markm-size-limits-take2 branch from d1b67fe to 1aae385 Compare August 28, 2022 19:51
@erights erights force-pushed the markm-far-classes branch from fcc3bb4 to e1a5527 Compare August 28, 2022 20:01
@erights erights force-pushed the markm-size-limits-take2 branch from 508e115 to 0af2e04 Compare August 28, 2022 20:02
@erights erights force-pushed the markm-far-classes branch from e1a5527 to 59c3cd8 Compare August 28, 2022 20:07
@erights erights force-pushed the markm-size-limits-take2 branch 4 times, most recently from 8b8a988 to 28d59b2 Compare August 29, 2022 01:38
@erights erights marked this pull request as ready for review August 29, 2022 01:44
@erights erights force-pushed the markm-far-classes branch from 195a02e to 5f94c72 Compare August 29, 2022 01:46
@erights erights marked this pull request as draft August 29, 2022 01:46
@erights erights marked this pull request as ready for review August 29, 2022 01:46
@erights erights force-pushed the markm-size-limits-take2 branch from e1e99b9 to ec3fba8 Compare September 1, 2022 04:06
@erights erights force-pushed the markm-size-limits-take2 branch from ec3fba8 to c14117a Compare September 1, 2022 18:43
@erights erights force-pushed the markm-size-limits-take2 branch from c14117a to d3c9e75 Compare September 2, 2022 22:01
@erights erights force-pushed the markm-size-limits-take2 branch from d3c9e75 to 8e36789 Compare September 3, 2022 22:16
Base automatically changed from markm-far-classes to master September 3, 2022 22:48
@erights erights force-pushed the markm-size-limits-take2 branch 2 times, most recently from 18ce081 to ec00bae Compare September 4, 2022 06:40
@erights erights changed the title WIP: patterns impose resource limits fix: patterns impose resource limits Sep 4, 2022
@erights erights force-pushed the markm-size-limits-take2 branch from 494e890 to e4357ce Compare September 4, 2022 19:38
erights
erights commented Sep 4, 2022
View reviewed changes
packages/store/test/test-pattern-limits.js Outdated Show resolved Hide resolved
packages/store/test/test-pattern-limits.js Outdated Show resolved Hide resolved
packages/store/test/test-pattern-limits.js Outdated Show resolved Hide resolved
@erights erights marked this pull request as ready for review September 4, 2022 19:42
@erights
Copy link
Member Author

erights commented Sep 4, 2022

This is now ready for review!

@dtribble dtribble self-requested a review September 5, 2022 17:57
@erights erights mentioned this pull request Sep 5, 2022
Closed
2 tasks
dtribble
dtribble approved these changes Sep 6, 2022
View reviewed changes
Copy link
Member

@dtribble dtribble left a comment

Choose a reason for hiding this comment

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

LGTM. Several comments about perf improvements and test improvements. I could not spot any issues.

Oh and some questions and suggestions around the initial values.

packages/store/src/patterns/patternMatchers.js Outdated Show resolved Hide resolved
packages/store/src/patterns/patternMatchers.js Show resolved Hide resolved
packages/store/src/patterns/patternMatchers.js Show resolved Hide resolved
packages/store/src/patterns/patternMatchers.js Show resolved Hide resolved
packages/store/src/patterns/patternMatchers.js Show resolved Hide resolved
packages/store/src/patterns/patternMatchers.js Outdated Show resolved Hide resolved
packages/store/src/patterns/patternMatchers.js Outdated Show resolved Hide resolved
packages/store/src/patterns/patternMatchers.js Show resolved Hide resolved
packages/store/src/patterns/patternMatchers.js Show resolved Hide resolved
packages/store/test/test-pattern-limits.js Outdated Show resolved Hide resolved
@erights erights force-pushed the markm-size-limits-take2 branch from e4357ce to d60d792 Compare September 6, 2022 03:30
@erights erights force-pushed the markm-size-limits-take2 branch from 24c30d4 to 68efd20 Compare September 6, 2022 21:38
This was referenced Sep 7, 2022
Open
Open
@erights erights added the automerge:squash Automatically squash merge label Sep 7, 2022
@mergify mergify bot merged commit 548c053 into master Sep 7, 2022
@mergify mergify bot deleted the markm-size-limits-take2 branch September 7, 2022 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtribble dtribble dtribble approved these changes

@arirubinstein arirubinstein Awaiting requested review from arirubinstein

@gibson042 gibson042 Awaiting requested review from gibson042

@warner warner Awaiting requested review from warner

Assignees

@erights erights

Labels
automerge:squash Automatically squash merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erights @dtribble