Speed up cros_config |
||
Issue descriptionRecent 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.
,
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
,
Dec 14 2017
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 |
||
Comment 1 by sjg@chromium.org
, Nov 15 2017Labels: Unibuild
Owner: sjg@chromium.org
Status: Assigned (was: Untriaged)