add some examples to the stdlib's documentation #11476

c-cube · 2022-08-04T01:58:28Z

No description provided.

stdlib/format.mli

gasche

Thanks! I think that this is an excellent idea. See inline comments.

stdlib/atomic.mli

gasche · 2022-08-04T08:20:36Z

stdlib/atomic.mli

+    # let () =
+      let threads = Array.init 8 (fun _ -> Thread.create read_file ()) in
+      Array.iter Thread.join threads;
+      Printf.printf "read %d bytes\n" (Atomic.get count_bytes_read)


I find it a bit awkward to exemplify concurrent atomics with a single-domain example that does not actually need atomics. Alternatives include using Domain directly (con: arguably too low-level in general) or domainslib (con: outside the stdlib, may change or get replaced in the medium-term future).

I've been testing it in a 4.14 repl on the side (in which it was surprisingly hard to get the Thread toplevel module?!), so I don't know exactly how I should go about that. Maybe I just need a 5.0-alpha switch to run this.

Domain.spawn should be on par in terms of complexity with Thread.create, right? Ignoring the ticker thread…

Another option is to show not a complete program with Domain.spawn, but just the code of a library function that may be called concurrently. (For example: a "unique id" generator that works by incrementing an internal counter.) You can show the sequential version, point out that it is not domain-safe, and move to Atomic instead.

moved to Domain. Hopefully someone can check the code, unless I get to trying 5.0-alpha on that.

stdlib/format.mli

stdlib/hashtbl.mli

stdlib/queue.mli

shindere · 2022-08-04T11:29:21Z

@pierreweis may be interested in reviewing this.

note: this is not tested, my current switch is still 4.14

dbuenzli · 2022-08-06T15:38:21Z

Could we please have the examples at the end of the modules in a dedicated section (e.g. {1:examples Examples}}) rather than in the module preamble (where a simple See {{!examples}examples}.} will do).

If you write them in the module preamble they get in the way when you peruse the API reference which what you eventually end up doing 99.9% of the time.

ulugbekna

Thanks for the PR! I left a couple of nits if that's okay :-)

stdlib/hashtbl.mli

ulugbekna · 2022-08-08T14:45:09Z

stdlib/atomic.mli

+    A basic use case is to have global counters that are updated in a
+    thread-safe way:


Wouldn't an even simpler example work? e.g., n domains each incrementing the counter k times by 1

I am just inclined to think that one shouldn't really need to understand how to work with files to understand how to use atomics.

I like to have "real" examples, I suppose. What do the maintainers think? @Octachron @gasche for example

I found this example a tad long as well, on the other hand the BFS sounded like a natural choice for queues. (I'm often too lazy to reach for a queue when I BFS and I use two lists, "frontier" and "next", but let's forget about this nitpick.) The Atomic example also had the issue of using Thread instead of Domain. It's fixed now (I haven't tried to run it yet, I'm in holidays until the end of the month and I somehow convinced myself that replying to messages is okay but running the compiler on PRs is not), but there is still a wide margin for debate. For example, the message we send is that Domain is the low-level, core module and that users should use higher-level abstractions (outside for the stdlib for now).

I think that the duty is on us to find a "real example" that is simpler but that @c-cube still likes, and maybe also manages to tiptoe around those difficulties.

stdlib/hashtbl.mli

ulugbekna · 2022-08-08T15:05:23Z

stdlib/queue.mli

+    ]}
+
+   For a more elaborate example, a classic algorithmic use of queues
+   is to implement a BFS (breadth-first search) through a graph.


BFS through a tree would be less involved?

I've never really needed a BFS through a tree. I think BFS through a graph (of some kind) is one of the main application of queues, so I illustrate with a real example. Same as above I suppose, it depends on what maintainers think the docs of the stdlib should look like.

dbuenzli · 2022-08-14T15:57:04Z

other modules would benefit too.

For Set and Map it's waiting for reviews in #11410

xavierleroy · 2022-08-14T16:06:40Z

For Set and Map it's waiting for reviews in #11410

Oh, I missed this one. Thanks for reminding me, and keep up the good work :-)

gasche · 2022-08-21T16:36:02Z

What do we need to move this PR forward?

I think that "restructuring the .mli file" could be split off to a separate PR. My understanding is that people would like a better/different example for the atomics, and are basically fine with the rest.

c-cube · 2022-08-21T17:23:40Z

I'm not sure what the simpler example could be. I agree that the IO stuff is a bit too long (even though it is, imho, a real use case for atomic counters: metrics).

Octachron · 2022-09-11T20:36:33Z

Concerning the examples in Atomic, I have the impression that it might work better to drop the first example and expand the second example to explain that the use of atomic guarantees that not elements are silently dropped from the queue.

c-cube · 2022-09-14T03:47:35Z

I expanded a bit the second example, and replaced the first one with a groundbreaking Proof Of Work™ implementation utilizing multiple cores to break Difficult Mathematical Problems™.

Basically just showing how to use bool Atomic.t to stop multiple threads; and a global thread-safe iteration counter. Both are best expressed with atomic.

gasche

I like the new first example of Atomic better, and overall I like the PR.

I think we should consider merging it; it's better to have examples than not, and I'm happy with the current ones. (We can always refine a bit later if we have concerns.)

Approved. (Note that stdilb PRs require two approvals.)

gasche · 2022-09-14T06:08:44Z

stdlib/atomic.mli

+    let () =
+      let criterion n = n <= 100 in
+      let threads =
+        Array.init 8


Instead of 8 you could use Domain.recommended_domain_count here. (This helps drive down the point that domains are not lightweight threads you can spawn as many as you want.)

gasche · 2022-09-14T06:41:05Z

stdlib/atomic.mli

+    (* find a number that satisfies [p], by... trying random numbers
+       until one fits. *)
+    let find_number_where (p:int -> bool) =
+      let rand = Random.State.make_self_init() in


Now that I became aware of splitting random generators, I would use a different approach where I seed a generator once at toplevel, and then split it repeatedly (on the main domain), passing a split state to find_number_where. This lets you easily control seeding (non-deterministic or deterministic are both possible). But the current approach is slightly simpler and also fine, this example is not about Random itself.

nojb

LGTM (modulo a few minor stylistic comments)

stdlib/format.mli

stdlib/hashtbl.mli

stdlib/moreLabels.mli

stdlib/queue.mli

stdlib/templates/hashtbl.template.mli

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

gasche · 2022-09-14T14:13:03Z

Changes

@@ -60,6 +60,9 @@ Working version
  halving memory usage while remaining tail-recursive.
  (Nicolás Ojeda Bär, review by Xavier Leroy and Gabriel Scherer)

+- #11476: Add examples in documentation of Hashtbl, Queue, Atomic, Format
+  (Simon Cruanes)


Could you complete the list of reviewers? (There is a "reviewers" list in the top right of the PR main webpage, which is accurate as far as I can tell.)

nojb · 2022-09-14T17:49:05Z

Merged, thanks!

c-cube · 2022-09-14T17:57:21Z

Thank you @nojb and to all the reviewers! 😁

c-cube added 3 commits August 3, 2022 21:58

add some examples to Format

ec4e7d2

cleanup a bit; ellipsis in long line

f9e6017

add examples to Queue

7db686d

c-cube changed the title ~~add some examples to Format~~ add some examples to the stdlib's documentation Aug 4, 2022

c-cube added 9 commits August 3, 2022 22:57

detail in queue

449755f

examples for Atomic

862318f

fix formatting for atomic

0149f66

atomic

87a0664

atomic

f8bc359

atomic

51cb192

some examples for Hashtbl

05c0469

fix sync issue

aa29d05

add changelog entry

eff4930

bluddy reviewed Aug 4, 2022

View reviewed changes

stdlib/format.mli Outdated Show resolved Hide resolved

c-cube added 3 commits August 3, 2022 23:59

hygiene

6f077a4

shorter example in Format

2a90633

indent

f665178

gasche reviewed Aug 4, 2022

View reviewed changes

c-cube added 6 commits August 4, 2022 20:13

remove ;;

b20db31

use Domain, not Thread, to demonstrate concurrent update to atomic

5088958

note: this is not tested, my current switch is still 4.14

less impersonal sentence

1374525

normal capitalization on "warning"

219b69f

optimize BFS

55746a5

hygiene

3359e40

ulugbekna reviewed Aug 8, 2022

View reviewed changes

c-cube added 3 commits August 9, 2022 20:06

remove repetition

97b8c51

clarify docs for Hashtbl, move examples at the end

3924b4c

move examples at the bottom of files; add sub-sections

caac61b

update examples for atomic

37e8dbb

c-cube added 2 commits September 13, 2022 23:51

missing comment

4c8f58b

doc

f4a138b

gasche approved these changes Sep 14, 2022

View reviewed changes

nojb approved these changes Sep 14, 2022

View reviewed changes

c-cube and others added 10 commits September 14, 2022 09:53

Update stdlib/format.mli

7670614

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Update stdlib/format.mli

e1076b7

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Update stdlib/format.mli

540d7ac

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Update stdlib/hashtbl.mli

892e350

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Update stdlib/templates/hashtbl.template.mli

83932d5

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Update stdlib/queue.mli

c5726d9

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Update stdlib/hashtbl.mli

944ca25

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Update stdlib/moreLabels.mli

6984548

Co-authored-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

rephrase sentence in Format

01f30d3

sync docs

f881dc6

gasche reviewed Sep 14, 2022

View reviewed changes

update changes to include reviewers

7b0db30

nojb merged commit e0afc0c into ocaml:trunk Sep 14, 2022

c-cube deleted the wip-doc-format branch September 14, 2022 17:57

gasche mentioned this pull request Nov 29, 2022

Add {In,Out}_channel to Stdlib #10545

Merged

gasche mentioned this pull request Jan 11, 2023

improving the {In,Out}_channel documentation #11883

Closed

hyphenrf mentioned this pull request Aug 1, 2023

Add examples to the Fun module #12452

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add some examples to the stdlib's documentation #11476

add some examples to the stdlib's documentation #11476

c-cube commented Aug 4, 2022

gasche left a comment

gasche Aug 4, 2022

c-cube Aug 4, 2022

gasche Aug 4, 2022

c-cube Aug 5, 2022

shindere commented Aug 4, 2022 via email

dbuenzli commented Aug 6, 2022 •

edited

ulugbekna left a comment

ulugbekna Aug 8, 2022

c-cube Aug 10, 2022

gasche Aug 10, 2022

ulugbekna Aug 8, 2022

c-cube Aug 10, 2022

dbuenzli commented Aug 14, 2022

xavierleroy commented Aug 14, 2022

gasche commented Aug 21, 2022

c-cube commented Aug 21, 2022

Octachron commented Sep 11, 2022

c-cube commented Sep 14, 2022

gasche left a comment

gasche Sep 14, 2022

gasche Sep 14, 2022

nojb left a comment

gasche Sep 14, 2022

nojb commented Sep 14, 2022

c-cube commented Sep 14, 2022

		A basic use case is to have global counters that are updated in a
		thread-safe way:

add some examples to the stdlib's documentation #11476

add some examples to the stdlib's documentation #11476

Conversation

c-cube commented Aug 4, 2022

gasche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shindere commented Aug 4, 2022 via email

dbuenzli commented Aug 6, 2022 • edited

ulugbekna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbuenzli commented Aug 14, 2022

xavierleroy commented Aug 14, 2022

gasche commented Aug 21, 2022

c-cube commented Aug 21, 2022

Octachron commented Sep 11, 2022

c-cube commented Sep 14, 2022

gasche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nojb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nojb commented Sep 14, 2022

c-cube commented Sep 14, 2022

dbuenzli commented Aug 6, 2022 •

edited