Skip to content

Prune time tracking#744

Open
wildsor wants to merge 7 commits into
Desbordante:mainfrom
wildsor:remove-time-accounting
Open

Prune time tracking#744
wildsor wants to merge 7 commits into
Desbordante:mainfrom
wildsor:remove-time-accounting

Conversation

@wildsor

@wildsor wildsor commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator

Currently there are four areas where time is tracked in the project:

  • ExecuteInternal return value
  • In some logging statements
  • Public methods in CIND/IND providing timing info
  • Algorithm timeouts

The ExecuteInternal value has long gone unused but people implement its calculation to comply with the interface. The CIND/IND public methods are not used anywhere, looks like they were part of some preliminary testing.
I don't think logging random timings provides any benefit to the user or to the developer. It looks like these logs were either used for some preliminary testing or cargo-culted from other algorithms. I decided to remove them.
Algorithm timeouts are technically part of the implementation and need a bit more thinking to get right, I decided to leave them as is for now.

@wildsor wildsor force-pushed the remove-time-accounting branch 3 times, most recently from 300d7c8 to 01cc36c Compare May 24, 2026 22:46
@wildsor wildsor changed the title Remove time accounting Remove time tracking May 24, 2026
@wildsor wildsor changed the title Remove time tracking Prune time tracking May 24, 2026
@wildsor wildsor marked this pull request as ready for review May 24, 2026 23:03
@wildsor wildsor added the python-packaging-risk There is a possibility of breaking the python packaging in PR. Enables additional build checks. label May 24, 2026
wildsor added 7 commits June 1, 2026 22:55
This is a holdover from the old codebase. Performance testing is now
separated from unit tests.
This is a holdover from when Desbordante was solely a benchmarking tool.
Right now this value is not used anywhere meaningful but people still
have to insert the useless time accounting logic to comply with the
interface.
These statements are intertwined with actual algorithm code, making it
more confusing to read. The statements are also not useful to a user of
the library. I suspect all of these were either leftovers from testing
code or were inserted because people thought it was important, given
that execution times were returned from Execute.
These values were not used anywhere. Looks like this stuff was
cargo-culted from other algorithms.
This function was only used for time accounting.
Removing time accounting made some execution methods have only a single
method call, the one actually doing the work. Moved the work directly to
the execution methods.
@wildsor wildsor force-pushed the remove-time-accounting branch from 01cc36c to 0a938f2 Compare June 1, 2026 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python-packaging-risk There is a possibility of breaking the python packaging in PR. Enables additional build checks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant