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 support for energy and heartbeats profiling #26788

Merged
merged 2 commits into from Jun 5, 2020
Merged

Remove support for energy and heartbeats profiling #26788

merged 2 commits into from Jun 5, 2020

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Jun 4, 2020

Both are disabled by default (energy at compile-time, heartbeats with a run-time option). Neither is tested of CI. Neither has been used in a long time. They might have Undefined Behavior: #26550 (comment). They each depend on a mostly-unmaintained C library. The thread-safety expectation of those libraries are unknown.

@jdm
Copy link
Member

jdm commented Jun 4, 2020

error[E0061]: this function takes 5 arguments but 7 arguments were supplied
    --> components/layout_thread/lib.rs:1453:13
     |
1453 |             profile_time::send_profile_data(
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 5 arguments
1454 |                 profile_time::ProfilerCategory::LayoutTextShaping,
     |                 -------------------------------------------------
1455 |                 self.profiler_metadata(),
     |                 ------------------------
1456 |                 &self.time_profiler_chan,
     |                 ------------------------
1457 |                 0,
     |                 -
1458 |                 text_shaping_time as u64,
     |                 ------------------------
1459 |                 0,
     |                 -
1460 |                 0,
     |                 - supplied 7 arguments
error: aborting due to previous error
For more information about this error, try `rustc --explain E0061`.
error: could not compile `layout_thread_2013`.
@jdm
jdm approved these changes Jun 4, 2020
```

The window size is also how many heartbeats will be stored in memory.

## Log Files

This comment has been minimized.

Copy link
@jdm

jdm Jun 4, 2020

Member

This whole file can be removed; it only documents the logs for heartbeat profiling.

@SimonSapin SimonSapin force-pushed the energy branch from 9d78e48 to 79d363a Jun 4, 2020
@SimonSapin
Copy link
Member Author

SimonSapin commented Jun 4, 2020

Fixed both and squashed

@jdm
Copy link
Member

jdm commented Jun 4, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2020

📌 Commit 79d363a has been approved by jdm

@highfive highfive assigned jdm and unassigned nox Jun 4, 2020
@SimonSapin
Copy link
Member Author

SimonSapin commented Jun 4, 2020

Updated unit tests for removed parameters.

@bors-servo r=jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2020

📌 Commit b282bd3 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jun 4, 2020

Testing commit b282bd3 with merge 98fe360...

@bors-servo
Copy link
Contributor

bors-servo commented Jun 5, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 98fe360 to master...

@bors-servo bors-servo merged commit 98fe360 into master Jun 5, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@bors-servo bors-servo deleted the energy branch Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.