Collect and log more timing information (issue #500) #523

veryordinally · 2022-09-12T15:03:31Z

Collect more timing info as per #500

Collect and log:

time elapsed for database commit
time elapsed for updating (=indexing to) database/btree in between commits

- Introduce `longform` flag for `list` subcommand, with following output format: For each output, we 1. Print outpoint and oldest sat contained 2. Followed by each ordinal range, indented by two spaces, and the size of the range

- time elapsed for database commit - time elapsed for updating (=indexing to) database/btree in between commits

- Allow multiple outpoints for list subcommand

Collect and log more timing information:

src/index.rs

casey · 2022-09-12T21:16:53Z

Since the logging and time calculation is somewhat complex, how about refactoring it into a method that we call twice, instead of duplicating:

fn commit(wtx: WriteTransaction, last_commit: Instant, height: u64) -> Instant {
  todo!()
}

veryordinally · 2022-09-13T18:42:16Z

fn commit(wtx: WriteTransaction, last_commit: Instant, height: u64) -> Instant {
todo!()
}

I had tried that but struggled with passing the WriteTransaction around. Will try once more.

veryordinally · 2022-09-13T19:21:09Z

I extracted a method as suggested. Not sure the way I handle the potential errors from commit() is OK like this. Realize that extracting methods gets tricky with the error handling approach.

Since the logging and time calculation is somewhat complex, how about refactoring it into a method that we call twice, instead of duplicating:
fn commit(wtx: WriteTransaction, last_commit: Instant, height: u64) -> Instant {
  todo!()
}

casey

Added some comments! I think you can use ? for error handling.

casey · 2022-09-13T21:21:19Z

src/index.rs

@@ -136,7 +137,7 @@ impl Index {
      let done = self.index_block(&mut wtx)?;

      if block % 1000 == 0 {
-        wtx.commit()?;
+        last_commit_time = Self::commit_and_log(wtx, last_commit_time, self.height()?.n()).unwrap();


Can you use ? for error handling? Also thinking that last_commit_time can be taking by &mut so it can just be updated in-place:

Suggested change

last_commit_time = Self::commit_and_log(wtx, last_commit_time, self.height()?.n()).unwrap();

Self::commit_and_log(wtx, &mut last_commit_time, self.height()?.n())?;

casey · 2022-09-13T21:21:41Z

src/index.rs

@@ -136,7 +137,7 @@ impl Index {
      let done = self.index_block(&mut wtx)?;

      if block % 1000 == 0 {
-        wtx.commit()?;
+        last_commit_time = Self::commit_and_log(wtx, last_commit_time, self.height()?.n()).unwrap();


Can you use ? for error handling? Also thinking that last_commit_time can be taking by &mut so it can just be updated in-place:

Suggested change

last_commit_time = Self::commit_and_log(wtx, last_commit_time, self.height()?.n()).unwrap();

Self::commit_and_log(wtx, &mut last_commit_time, self.height()?.n())?;

casey · 2022-09-13T21:23:51Z

src/index.rs

@@ -147,11 +148,28 @@ impl Index {
      block += 1;
    }

-    wtx.commit()?;
+    Self::commit_and_log(wtx, last_commit_time, self.height()?.n()).unwrap();


Suggested change

Self::commit_and_log(wtx, last_commit_time, self.height()?.n()).unwrap();

Self::commit_and_log(wtx, &mut last_commit_time, self.height()?.n())?;

casey · 2022-09-13T21:25:02Z

src/index.rs


    Ok(())
  }

+  fn commit_and_log(
+    wtx: WriteTransaction,
+    last_commit_time: Instant,


Suggested change

last_commit_time: Instant,

last_commit_time: &mut Instant,

casey · 2022-09-13T21:26:41Z

src/index.rs

+      height,
+      (now - commit_start_time).as_millis()
+    );
+    Ok(Instant::now())


Suggested change

Ok(Instant::now())

*last_commit_time = Instant::now();

Ok(())

casey · 2022-09-13T21:28:17Z

src/index.rs

+    wtx: WriteTransaction,
+    last_commit_time: Instant,
+    height: u64,
+  ) -> Result<Instant> {


Suggested change

) -> Result<Instant> {

) -> Result {

casey · 2022-09-13T21:28:52Z

src/subcommand/list.rs

@@ -2,22 +2,36 @@ use super::*;

 #[derive(Debug, Parser)]
 pub(crate) struct List {
-  outpoint: OutPoint,
+  #[clap(long, short, help = "Use extended output format")]


I think the extended output format code snuck in here.

Ups, yes. I must have messed something up after I had merged those changes into my master.

veryordinally and others added 4 commits September 12, 2022 09:57

- Allow multiple outpoints for list subcommand

960fcd0

- Introduce `longform` flag for `list` subcommand, with following output format: For each output, we 1. Print outpoint and oldest sat contained 2. Followed by each ordinal range, indented by two spaces, and the size of the range

Collect and log more timing information:

35f61a5

- time elapsed for database commit - time elapsed for updating (=indexing to) database/btree in between commits

Merge pull request #1 from VirtuallyThere/list-multiple-outputs

c3d1f8f

- Allow multiple outpoints for list subcommand

Merge pull request #2 from VirtuallyThere/collect-timing-issue-500

611b1db

Collect and log more timing information:

casey reviewed Sep 12, 2022

View reviewed changes

src/index.rs Outdated Show resolved Hide resolved

Extract method for committing, timing and logging

9ab9d7d

casey requested changes Sep 13, 2022

View reviewed changes

veryordinally closed this by deleting the head repository Oct 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collect and log more timing information (issue #500) #523

Collect and log more timing information (issue #500) #523

veryordinally commented Sep 12, 2022

casey commented Sep 12, 2022

veryordinally commented Sep 13, 2022 •

edited

Loading

veryordinally commented Sep 13, 2022

casey left a comment

casey Sep 13, 2022

casey Sep 13, 2022

casey Sep 13, 2022

casey Sep 13, 2022

casey Sep 13, 2022

casey Sep 13, 2022

casey Sep 13, 2022

veryordinally Sep 14, 2022

	last_commit_time = Self::commit_and_log(wtx, last_commit_time, self.height()?.n()).unwrap();
	Self::commit_and_log(wtx, &mut last_commit_time, self.height()?.n())?;

	Self::commit_and_log(wtx, last_commit_time, self.height()?.n()).unwrap();
	Self::commit_and_log(wtx, &mut last_commit_time, self.height()?.n())?;

Collect and log more timing information (issue #500) #523

Collect and log more timing information (issue #500) #523

Conversation

veryordinally commented Sep 12, 2022

casey commented Sep 12, 2022

veryordinally commented Sep 13, 2022 • edited Loading

veryordinally commented Sep 13, 2022

casey 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veryordinally commented Sep 13, 2022 •

edited

Loading