Skip to content

[RFC] Trace recording in tezos#16

Open
Ngoguey42 wants to merge 1533 commits intotarides:masterfrom
Ngoguey42:new-action-trace-recording
Open

[RFC] Trace recording in tezos#16
Ngoguey42 wants to merge 1533 commits intotarides:masterfrom
Ngoguey42:new-action-trace-recording

Conversation

@Ngoguey42
Copy link

@Ngoguey42 Ngoguey42 commented Oct 5, 2021

The commits should help you understand the refactorings that I made on existing code. Every commit from and after lib_context: Restore compilation compile.

@Ngoguey42 Ngoguey42 force-pushed the new-action-trace-recording branch from 8f90c92 to 277ffbb Compare October 5, 2021 12:18
type t = context

module type S = Tezos_context_sigs.Context.S
module type S = Tezos_context_sigs.Context.MEM
Copy link

Choose a reason for hiding this comment

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

Why did you rename this? This not the signature of the read-only backend :-)

Copy link
Author

@Ngoguey42 Ngoguey42 Oct 6, 2021

Choose a reason for hiding this comment

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

I made Tezos_context_sigs.Context.S be the signature of the full lib_context implementation (which used to be lib_context/context.mli).

I can look for a better naming than MEM for the old Tezos_context_sigs.Context.S

{Logs.report}

let index_log_size = ref None
let index_log_size = Env.(v.index_log_size)
Copy link

Choose a reason for hiding this comment

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

This commit (d51fbed) should be upstreamed separately (it's a good change on its own).

Copy link

Choose a reason for hiding this comment

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

(also for only accessing fields, I would write it as v.Env.index_log_size and so on)

Copy link
Author

Choose a reason for hiding this comment

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

(also for only accessing fields, I would write it as v.Env.index_log_size and so on)

v is in Env, so v.Env wouldn't work.

Do you think I should rework the Env module?

Copy link

@samoht samoht Oct 6, 2021

Choose a reason for hiding this comment

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

v is in Env, so v.Env wouldn't work.

Do you think I should rework the Env module?

Ha yes sorry I was confused. Is there a way to not use a global variable in Env (and thread a local paramater around instead?)

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I can get rid of the global variable and replace it with a Env.get : unit -> Env.t thunk that derives Sys.argv each time it is called.

Copy link

Choose a reason for hiding this comment

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

that's a good idea only if it is called only once :-)

Copy link
Author

Choose a reason for hiding this comment

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

It would get called multiple times. I'm not sure how to do otherwise :(

Copy link

Choose a reason for hiding this comment

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

can you add that env value to Context.t ? if not, don't worry too much and keep a global state.

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately the env needs to be read before a Context.t is instantiated

Copy link

Choose a reason for hiding this comment

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

as long as it's read once it's ok :-)

@Ngoguey42 Ngoguey42 force-pushed the new-action-trace-recording branch from 277ffbb to f7f6cd0 Compare October 27, 2021 12:16
@Ngoguey42 Ngoguey42 force-pushed the new-action-trace-recording branch 2 times, most recently from 0052b42 to 69c2de3 Compare December 10, 2021 13:48
@Ngoguey42 Ngoguey42 force-pushed the new-action-trace-recording branch 3 times, most recently from 965a643 to a57a18c Compare January 11, 2022 17:29
@maiste maiste force-pushed the new-action-trace-recording branch from a57a18c to 2e1586c Compare January 25, 2022 10:03
@Ngoguey42 Ngoguey42 force-pushed the new-action-trace-recording branch from 2e1586c to b25d227 Compare February 16, 2022 13:56
arvidj and others added 19 commits February 22, 2022 13:12
Splits the protocol unit test jobs over multiple runners.

Furthermore, tweak the `test_script_wrapper.sh` and the Makefile
targets using it so that dune is invoked once with several targets,
instead of one invocation per target. Thus, dune parallelism is
exploited.
CI/Tests: manually balance unit tests

See merge request tezos/tezos!4057
Add selected_snapshot RPC

Closes #2485

See merge request tezos/tezos!4479
Proto: compile the protocol environment without Stdlib (fixes #2209)

Closes #2209

See merge request tezos/tezos!4482
Tx_rollup,Proto: remove fixme

See merge request tezos/tezos!4531
Marge Bot and others added 13 commits March 2, 2022 09:31
Proto,Tx_rollups: Inbox message count limit

See merge request tezos/tezos!4548
doc: explicit RPC path prefixes

See merge request tezos/tezos!4570
Tx rollup,Proto: clean empty balance in the context.

Closes #2495

See merge request tezos/tezos!4594
Proto: fix https://gitlab.com/tezos/tezos/-/issues/2084

Closes #2554 and #2084

See merge request tezos/tezos!4457
…ster'

Add tzip info about backwards compatibility to the doc

See merge request tezos/tezos!4544
Tx_rollup: fix typos and constants in python

See merge request tezos/tezos!4566
@Ngoguey42 Ngoguey42 force-pushed the new-action-trace-recording branch 2 times, most recently from 3da7ee3 to 9e3ba2f Compare March 2, 2022 14:50
@Ngoguey42 Ngoguey42 force-pushed the new-action-trace-recording branch from 9e3ba2f to fb757a2 Compare March 3, 2022 16:37

let init ?patch_context:user_patch_context_opt ?readonly
(* ?indexing_strategy *) x =
?(indexing_strategy = `Minimal) x =
Copy link
Author

Choose a reason for hiding this comment

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

No need to put the minimal here

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.