New issue
Advanced search Search tips

Issue 887254 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocking:
issue 746956



Sign in to add a comment

Support jumbo in //device/bluetooth

Project Member Reported by brat...@opera.com, Sep 20

Issue description

The code in device/bluetooth takes about 6 CPU minutes to build in official jumbo build tests. That makes it worth taking a look to see if it is worth jumbofying it and possibly save 1% of the jumbo compile time.

 
Blocking: 746956
If no one has started yet, can I start it?
Components: IO>Bluetooth Build OS>Systems>Bluetooth
Sure. Note that this may take some code changes. Long ago I took a quick look and listed these files as not jumbo compatible (may have changed):

 jumbo_excluded_sources = [
    # Symbols
    "bluetooth_adapter_factory_wrapper.cc",
  ]

  if (is_mac) {
    jumbo_excluded_sources += [
      # pointer is missing a nullability type specifier
      "bluetooth_low_energy_central_manager_delegate.mm",
      "bluetooth_low_energy_peripheral_delegate.mm",
    ]
  }

  if (is_win) {
    jumbo_excluded_sources += [
      # Doing magic with multiple inclusions of devpkey.h to either
      # use or generate GUID things (INITGUID)
      "bluetooth_low_energy_defs_win.cc",
    ]
  }

    jumbo_excluded_sources += [
      # Symbols
      "bluetooth_pairing_winrt.cc",  # PostTask same as in bluetooth_device_wintt.cc
    ]


if (is_chromeos || is_linux) {
      jumbo_excluded_sources += [
        # Symbols
        "dbus/bluetooth_gatt_characteristic_service_provider.cc",
        "dbus/bluetooth_gatt_characteristic_service_provider_impl.cc",
        "dbus/bluetooth_gatt_descriptor_service_provider_impl.cc",
        "dbus/bluetooth_le_advertisement_service_provider.cc",
        "dbus/bluetooth_media_transport_client.cc",
        "dbus/fake_bluetooth_device_client.cc",
        "dbus/fake_bluetooth_gatt_descriptor_service_provider.cc",
        "bluez/bluetooth_gatt_descriptor_bluez.cc",
        "bluez/bluetooth_remote_gatt_descriptor_bluez.cc",
      ]
}


You may rediscover that as you go along.
For the INITGUID case mentioned above, I don't know of any way to solve that by jumbo_excluded_sources so let me know if you figure something out.
Labels: -Type-Bug Type-Feature
Owner: brat...@opera.com
Status: Assigned (was: Untriaged)
Assigning to bratell@ because codeimpl@ is not a project member and cannot be assigned issues.
Components: -OS>Systems>Bluetooth
@bratell
Thanks for your detail guide :)
When I tested it with below code, dry run was passed.
(https://chromium-review.googlesource.com/c/chromium/src/+/1256325)
I doubt whether I should trust the results for Windows.
I haven't built for window because I have Linux & Mac machine without Window machine.
Did you "find bluetooth_low_energy_defs_win.cc" and "bluetooth_pairing_winrt.cc" when you build chromium at window machine?

  jumbo_excluded_sources = [
    #Symbol
    "bluetooth_adapter_factory_wrapper.cc",
  ]

  if (is_chromeos || is_linux) {
    if (use_dbus) {
      jumbo_excluded_sources += [
        "bluez/bluetooth_remote_gatt_descriptor_bluez.cc",
        "dbus/bluetooth_gatt_characteristic_service_provider_impl.cc",
        "dbus/bluetooth_media_transport_client.cc",
        "dbus/fake_bluetooth_device_client.cc",
        "dbus/fake_bluetooth_gatt_descriptor_service_provider.cc",
        "dbus/bluetooth_gatt_descriptor_service_provider_impl.cc",
        "dbus/bluetooth_le_advertisement_service_provider.cc",
      ]
    }
  }

  if (is_mac) {
    jumbo_excluded_sources += [
      "bluetooth_low_energy_central_manager_delegate.mm",
      "bluetooth_low_energy_peripheral_delegate.mm",
    ]
  }
Good, then you know that it can work. Now jumbo_excluded_sources is just a last resort, to use when there is no reasonable other fix so it cannot land like that. The next step is to remove lines from jumbo_excluded_sources, see what breaks, and figure out a way to avoid that.
I can help with Windows later if that is all that remains.
And thanks for looking at this!
Cc: codei...@gmail.com
Thanks for the advice and encouragement.
As you told me, I will try to remove lines in jumbo_excluded_sources, see what breaks, and figure out a way to avoid that!
Really Thanks.
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40837253e16021261f2e15fb9773327eefd7103d

commit 40837253e16021261f2e15fb9773327eefd7103d
Author: Byoungkwon Ko <codeimpl@gmail.com>
Date: Wed Nov 21 07:40:30 2018

[bluetooth] Renamed global variables in device/bluetooth

In jumbo build experiments, files get merged and variables and
functions with the same name may end up in the same namespace/scope
and conflict. This happens for g_singleton in :
bluetooth_adapter_factory.cc
bluetooth_adapter_factory_wrapper.cc

[1] https://chromium.googlesource.com/chromium/src/+/lkgr/docs/jumbo.md

Bug: 887254
Change-Id: I97ff48a6aae71be0b935db1f7623da7ef65bac97
Reviewed-on: https://chromium-review.googlesource.com/c/1325109
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Byoungkwon Ko <codeimpl@gmail.com>
Cr-Commit-Position: refs/heads/master@{#609940}
[modify] https://crrev.com/40837253e16021261f2e15fb9773327eefd7103d/device/bluetooth/bluetooth_adapter_factory_wrapper.cc

Sign in to add a comment