Closed Bug 1612251 Opened 6 months ago Closed 3 months ago

Use Executors.newSingleThreadScheduledExecutor instead of Thread.sleep for Java Sampler

Categories

(Core :: Gecko Profiler, enhancement, P2)

Unspecified
Android
enhancement

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

References

Details

Attachments

(1 file)

Actually, we use Thread.sleep to get next sampling duration. But as long as old comment, this sampling min duration is 10ms (see https://searchfox.org/mozilla-central/rev/ad6234148892bc29bf08d736604ac71925138040/tools/profiler/core/platform.cpp#4192).

When I test Executors.newSingleThreadScheduledExecutor on old device (10 year's old Galaxy Nexus) instead of Thread.sleep, it can change to 1-2ms.

So we should use ScheduledExecutorService instead.

Hey Nazim, I think this is relevant to the other bug that aims to put the interval back a 1ms. It would make sense to tackle both bugs together. What do you think?

Component: Performance Tools (Profiler/Timeline) → Gecko Profiler
Flags: needinfo?(canaltinova)
Product: DevTools → Core

Yeah, that's a good idea! We can land both of them together.

Makoto, it looks assigned to you. Do you mind if I take this one, if you are not working on it already?

Flags: needinfo?(canaltinova) → needinfo?(m_kato)
Priority: -- → P2

(In reply to Nazım Can Altınova [:canova][:canaltinova on phabricator] from comment #2)

Yeah, that's a good idea! We can land both of them together.

Makoto, it looks assigned to you. Do you mind if I take this one, if you are not working on it already?

I already have a patch, so it will be ready for review soon.

Hey makoto, try run seems green! Do you think you can push your patch for review?

Actually, we use Thread.sleep to get next sampling duration. But as long as
old comment, this sampling min duration is 10ms. But when I test
Executors.newSingleThreadScheduledExecutor on old device (10 year's old
Galaxy Nexus) instead of Thread.sleep, it can use 2ms sampling duration.

So we should use ScheduledExecutorService instead.

Flags: needinfo?(m_kato)
Attachment #9139514 - Attachment description: Bug 1612251 - Use Executors.newSingleThreadScheduledExecutor to support 2ms sampling. → Bug 1612251 - Use Executors.newSingleThreadScheduledExecutor to support 1ms sampling.
Attachment #9139514 - Attachment description: Bug 1612251 - Use Executors.newSingleThreadScheduledExecutor to support 1ms sampling. → Bug 1612251 - Use Executors.newSingleThreadScheduledExecutor to support <10ms sampling.
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/1e06eab47c7a
Use Executors.newSingleThreadScheduledExecutor to support <10ms sampling. r=julienw
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
You need to log in before you can comment on or make changes to this bug.