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

Source Mapper Improvements: Better Performance and Its Consequences #677

Open
4 of 15 tasks
tzaffi opened this issue Feb 23, 2023 · 1 comment
Open
4 of 15 tasks

Source Mapper Improvements: Better Performance and Its Consequences #677

tzaffi opened this issue Feb 23, 2023 · 1 comment
Labels
new-feature-request Feature request that needs triage Team Scytale

Comments

@tzaffi
Copy link
Contributor

tzaffi commented Feb 23, 2023

Problem

The source mapper is very slow and a memory hog. As a result, two non-ideal artifacts have been added:

  1. pyteal.ini which defaults to pyteal-source-mapper.enabled = False
  2. Nightly tests in C.I. which carve out the slowest portion of the tests and complicate local testing

Solution

@jasonpaulos has an experimental approach which promises to speed things up to the point that these artifacts can be eliminated, or mitigated.

I propose:

  • nightly tests be eliminated once it is established that all tests can be run in a reasonable amount of time (however Nightly build #630 should be taken into account before eliminating)
  • Default the sourcemapper to enabled = True in the pyteal.ini but don't get rid of it

Why keep the pyteal.ini ?

  • It may still be useful to allow dev's to turn the feature off completely.
  • We may desire to feature gate (cf. Prototype: Feature gate support #601 ) via pyteal.ini. However, a more modern approach uses pyproject.toml instead.

Tasks

  • Improvements to NatalStackFrame and StackFrame
    • only keep the "best" frame, not a list of best frames (which in actuality always has length 1)
    • incorporate ideas from @jasonpaulos branch and this S.O. post
    • investigate saving memory by using slots (e.g.: nice example )
    • investigate replacing the debug-only member StackFrame.full_stack by StackFrame._debug_frame_origin and possibly add a method StackFrame._debug_full_stack() which can recreate this information
    • modify the unit tests that employ NatalStackFrame._keep_all_debugging so that they can work on the new lighter frame without keeping the entire stack. These are the following:
      • tests/unit/sourcemap_test.py::test_frames
      • tests/unit/sourcemap_monkey_unit_test.py::test_frame_info_is_right_before_core_last_drop_idx
  • Nightly tests
    • bring back python 3.10 source map tests into the "On Commit" workflow
    • either remove the nightly workflow, or repurpose it
  • pyteal.ini (addressed in Feature Gates without a Config File #687 and Best Guess Sourcemapping when Partially Enabled #688)
    • decide whether to keep it and/or use pyproject.toml instead (DECISION: no file-based configs)
    • if keeping, default source mapping to enabled
    • or maybe support both: Adopt pytest's approach for dealing with both *.ini and pyproject.toml files. In particular, in the case that pyteal.ini isn't found, look also for pyproject.toml, and then for the section [tool.pyteal.ini] Such a section should look like:
[tool.pyteal.ini]
source-mapper-enabled = true
source-mapper-debug = false

Dependencies

None

Urgency

Medium - defaulting to pyteal-source-mapper.enabled = True will make source mapping a lot more accessible, and getting rid of nightly tests will reduce developer friction.

@tzaffi tzaffi added Team Scytale new-feature-request Feature request that needs triage labels Feb 23, 2023
@tzaffi tzaffi mentioned this issue Feb 23, 2023
43 tasks
@bbroder-algo
Copy link
Contributor

I like the pyproject.toml-based categorized feature gate support approach over adding a pyteal.ini.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature-request Feature request that needs triage Team Scytale
Projects
None yet
Development

No branches or pull requests

2 participants