New issue
Advanced search Search tips

Issue 779439 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Speed up cros_config

Project Member Reported by sjg@chromium.org, Oct 30 2017

Issue description

Recent benchmarking suggests it takes 7ms for cros_config to read a config item, plus 10ms for mosys. We can probably eliminate the mosys time, but need to reduce the cros_config time.

Options:
- Reduce run-time config size by removing things we don't need
- Stop using mosys by writing identity info into a read-only mount at boot (adurbin's idea)
- Write all config for the current model/submodel in the same way ( (adurbin's idea))
- See if there is something slow about the C++ libraries we are using (base::, etc.)

Goal should be <1ms.
 

Comment 1 by sjg@chromium.org, Nov 15 2017

Cc: jclinton@chromium.org shapiroc@chromium.org
Labels: Unibuild
Owner: sjg@chromium.org
Status: Assigned (was: Untriaged)
A new test on robo shows it takes about 20ms at present :-(

time (for i in $(seq 0 1000); do cros_config / brand-code >/dev/null 2>&1;  done)

real	0m20.194s
user	0m8.871s
sys	0m7.695s

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/39d403b2e233129859c39cd198182addd7f43e1c

commit 39d403b2e233129859c39cd198182addd7f43e1c
Author: Simon Glass <sjg@chromium.org>
Date: Thu Dec 14 23:27:04 2017

chromeos-config: Read device identity without mosys

It is unfortunate that it is so hard to read device identity on x86
machines. On ARM machines this is available by looking in
'/proc/device-tree/compatible' for a matching string. On x86 we have to
decode 'SMBIOS' tables, a very low-level data structure defined by a
specification. The tables are set up by AP firmware and contain a large
amount of information, from which only two small pieces are relevant for
determining device identify.

To date our approach has been to call out to mosys to obtain this
information on x86, since it has a decoder for SMBIOS tables. But this has
various problems:

- It is very slow, doubling the run-time of cros_config to about 20ms
- It introduces a dependency in cros_config on mosys, which itself reads
     the master configuration. This is not a circular dependendency, since
     mosys depends only on the data, not libcros_config, but it has proved
     to be confusing.
- It puts us at the mercy of however mosys implements its identity reading
     (e.g. the meaning of 'mosys platform name'), although we have
     mitigated this by using a common implementation for all unibuild
     boards
- It is in conflict with the goal of dropping use of 'mosys platform' with
     unified builds ( crbug.com/770768 ) and in fact heads in the other
     direction
- Mosys has essentially no tests, although we have managed to add some for
     some of the parts that cros_config currently uses

A close look at the SMBIOS tables shows that they are fairly easy to parse
for the tiny pieces of info we need in cros_config. On ARM platforms it
would make no sense to call out to mosys for something we can obtain from
a file. So we may as well implement reading SMBIOS tables for x86.

Add a new identity implementation, which handles reading identity. For
testing, adjust the test to write a fake identity file and have
libcros_config read that, so that we get good test coverage over the
implementation.

Drop use of mosys for identity. This results in doubling the speed of
cros_config to 10ms. This is still way too slow, but if we only call it
20 times on start-up (and not in the critical path) it is getting closer
to being acceptable.

BUG= chromium:779439 
TEST=FEATURES=test emerge-coral --nodeps  chromeos-config-tools
Manual test on coral

Change-Id: Ie913c78aab1116e28211d1d03dfc4e58ffb89811
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/774904

[modify] https://crrev.com/39d403b2e233129859c39cd198182addd7f43e1c/chromeos-config/cros_config_main.cc
[modify] https://crrev.com/39d403b2e233129859c39cd198182addd7f43e1c/chromeos-config/libcros_config/cros_config.cc
[modify] https://crrev.com/39d403b2e233129859c39cd198182addd7f43e1c/init/chromeos_startup
[add] https://crrev.com/39d403b2e233129859c39cd198182addd7f43e1c/chromeos-config/libcros_config/identity.cc
[modify] https://crrev.com/39d403b2e233129859c39cd198182addd7f43e1c/chromeos-config/libcros_config/cros_config_common.cc
[modify] https://crrev.com/39d403b2e233129859c39cd198182addd7f43e1c/chromeos-config/chromeos-config.gyp
[modify] https://crrev.com/39d403b2e233129859c39cd198182addd7f43e1c/chromeos-config/libcros_config/cros_config.h

Comment 3 by sjg@chromium.org, Dec 14 2017

Status: Fixed (was: Assigned)
This is a good speed up and I'm closing this bug for now.

I'm going to add a kernel driver, tracked in a separate bug  crbug.com/795122 

Sign in to add a comment