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

Remove telemetry #439

Merged
merged 1 commit into from
Feb 2, 2024
Merged

Remove telemetry #439

merged 1 commit into from
Feb 2, 2024

Conversation

tinchodias
Copy link
Collaborator

The purpose of Telemetry is inspecting details of the execution of a space.

Why removing it?

Reason 1: it obscures the Bloc code by this kind of permanent instrumentation. For example:

			self telemetry
				timeSync: [ 'Layout ', self class name ]
				during: [ self onLayout: aRectangle context: aBlElementBoundsUpdateContext ] ].

instead of just having self onLayout: aRectangle context: aBlElementBoundsUpdateContext. When the telemetry is enabled, the time to run the block is measured, and emitted in a beacon signal to be logged.

Reason 2: the telemetry infrastructure involves about 11 classes that contributes to make Bloc harder to comprehend to new users, while general profiling tools and programmer's creativity are enough to get the same conclusions. It may be that stubborn or rustic and I'm missing something important, but I don't find the need for this special tool to profile Bloc.

So, I propose to replace it by this methodology:

  • Using the general Pharo Profiler to analyze performance along a period of time.
  • Inserting ad-hoc instrumentation profiling code, in cases that require catching a hiccup in some frame of an animation.

Copy link
Member

@jecisc jecisc left a comment

Choose a reason for hiding this comment

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

I agree with the mail you sent on bloc ML

@plantec
Copy link
Collaborator

plantec commented Feb 2, 2024

please go!

@jecisc jecisc merged commit 967978f into dev-1.0 Feb 2, 2024
12 checks passed
@tinchodias tinchodias deleted the removeTelemetry branch February 7, 2024 06:00
@tinchodias tinchodias mentioned this pull request Feb 21, 2024
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.

3 participants