Only load attributes relevant to the product's current attribute set

[jameshalsall]

Description

This is a port of the fix suggested in #13308 (comment). Currently Magento will load every attribute in the system when creating / updating a product, instead it should only load attributes that are in the product's attribute set.

Consider the following setup:

• 1000 attribute sets
• 100 attributes in each set

Magento would load 100,000 attributes, causing a huge increase in the number of queries to the database.

After this change Magento will only load 100 attributes for a product because there are 100 in each attribute set. It also has no impact on smaller set ups.

We observed a ~10x performance boost in the above example setup when creating products via the REST API.

Fixed Issues (if relevant)

N/A

Manual testing scenarios

1. Create example set up as above
2. Test creating a product via REST API on Magento 2.2
3. Re-test creating a product with this code and observe performance increase.

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[mslabko]

@jameshalsall , Thank you for PR!
I've run Performance tests on medium profile with your changes and will update you with results tomorrow

[mslabko]

Average improvement for all "product" scenarios is about 50%.
Here is example from Catalog browsing scenario:

Open Category                                   641.00ms   -38.8%     (-407ms)   improvement
Simple Product 1 View                       406.00ms   -55.8%     (-512ms)   improvement
Simple Product 1 Add To Cart            237.00ms   -67.9%     (-502ms)   improvement
Get Configurable Product Options    122.00ms   -81.0%     (-520ms)   improvement

Great improvement, Thank you, @jameshalsall !

[mslabko]

I've reviewed these changes and find out, that we apply not an ideal solution in #13308 which leads us to use also not ideal decisions for fix performance issues. Here are details:
For now, we have 3 alternatives for retrieve EAV attributes coded:

1. existing \Magento\Eav\Model\Config::getEntityAttributes (API)
2. introduced GetCustomAttributeCodesInterface (new API), which depends on legacy MetadataServiceInterface
3. introduced changes within this PR "retrieving by attribute set"

Due to all 3 variants are used in our code base this is potential back-door for bugs.

The problem of (2) approach is in using legacy MetadataServiceInterface which not take into account, that entity attributes is belong to attribute set
The problem of (3) is in mixing MetadataServiceInterface and SearchCriteria approaches

I can give 2 recomendations

1. ideally is to eliminate interface GetCustomAttributeCodesInterface and use \Magento\Eav\Model\Config::getEntityAttributes directly in Entity Model. I do not see a strong reason for introducing new API and it looks like we have no cases where this API can be used separately from Entity Model.
   Please note, that this recommendation will keep the main idea of Improve getCustomAttribute() performance #13308 and make it performance-friendly.

2. Left GetCustomAttributeCodesInterface, but remove MetadataServiceInterface, so make it like: GetCustomAttributeCodesInterface::execute(int $attributeSetId)

Please, note, these recommendations are not mandatory for this PR (it can be processed as is with current changes and in the feature, but until 2.3.0 we can implement proposed changes)

[ishakhsuvarov]

Hi @jameshalsall
We have executed internal CICD tests on this Pull Request and encountered multiple api-functional test failures, I'm attaching the list just in case:

Magento.Bundle.Api.ProductServiceTest.testUpdateBundleModifyExistingSelection Magento.Bundle.Api.ProductServiceTest.testUpdateBundleModifyExistingOptionOnly Magento.Bundle.Api.ProductServiceTest.testUpdateProductWithoutBundleOptions Magento.Bundle.Api.ProductServiceTest.testUpdateBundleAddSelection Magento.Bundle.Api.ProductServiceTest.testUpdateBundleAddAndDeleteOption Magento.Bundle.Api.ProductServiceTest.testUpdateBundleModifyExistingSelection Magento.Bundle.Api.ProductServiceTest.testUpdateBundleModifyExistingOptionOnly Magento.Bundle.Api.ProductServiceTest.testUpdateProductWithoutBundleOptions Magento.Bundle.Api.ProductServiceTest.testUpdateBundleAddSelection Magento.Bundle.Api.ProductServiceTest.testUpdateBundleAddAndDeleteOption Magento.Catalog.Api.ProductRepositoryInterfaceTest.testProductOptions Magento.Catalog.Api.ProductRepositoryInterfaceTest.testProductWithMediaGallery Magento.Catalog.Api.ProductRepositoryInterfaceTest.testEavAttributes Magento.Catalog.Api.ProductRepositoryInterfaceTest.testTierPrices Magento.Catalog.Api.ProductRepositoryInterfaceTest.testUpdateProductCategoryLinksNullOrNotExists Magento.Catalog.Api.ProductRepositoryInterfaceTest.testUpdateProductCategoryLinksPosistion Magento.Catalog.Api.ProductRepositoryInterfaceTest.testUpdateProductCategoryLinksUnassign Magento.Catalog.Api.ProductRepositoryInterfaceTest.testProductOptions Magento.Catalog.Api.ProductRepositoryInterfaceTest.testProductWithMediaGallery Magento.Catalog.Api.ProductRepositoryInterfaceTest.testEavAttributes Magento.Catalog.Api.ProductRepositoryInterfaceTest.testTierPrices Magento.Catalog.Api.ProductRepositoryInterfaceTest.testUpdateProductCategoryLinksNullOrNotExists Magento.Catalog.Api.ProductRepositoryInterfaceTest.testUpdateProductCategoryLinksPosistion Magento.Catalog.Api.ProductRepositoryInterfaceTest.testUpdateProductCategoryLinksUnassign Magento.Catalog.Api.ProductRepositoryMultiCurrencyTest.testGet Magento.Catalog.Api.ProductRepositoryMultiCurrencyTest.testGet Magento.CatalogInventory.Api.ProductRepositoryInterfaceTest.testCatalogInventory Magento.CatalogInventory.Api.ProductRepositoryInterfaceTest.testCatalogInventoryWithBogusData Magento.CatalogInventory.Api.ProductRepositoryInterfaceTest.testUpdatingQuantity Magento.CatalogInventory.Api.ProductRepositoryInterfaceTest.testCatalogInventory Magento.CatalogInventory.Api.ProductRepositoryInterfaceTest.testCatalogInventoryWithBogusData Magento.CatalogInventory.Api.ProductRepositoryInterfaceTest.testUpdatingQuantity Magento.ConfigurableProduct.Api.ProductRepositoryTest.testDeleteConfigurableProductOption Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductOption Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductLinks Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductLinksWithNonExistingProduct Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductLinksWithDuplicateAttributes Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductLinksWithWithoutVariationAttributes Magento.ConfigurableProduct.Api.ProductRepositoryTest.testDeleteConfigurableProductOption Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductOption Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductLinks Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductLinksWithNonExistingProduct Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductLinksWithDuplicateAttributes Magento.ConfigurableProduct.Api.ProductRepositoryTest.testUpdateConfigurableProductLinksWithWithoutVariationAttributes Magento.Downloadable.Api.ProductRepositoryTest.testUpdateDownloadableProductLinks Magento.Downloadable.Api.ProductRepositoryTest.testUpdateDownloadableProductLinksWithNewFile Magento.Downloadable.Api.ProductRepositoryTest.testUpdateDownloadableProductSamples Magento.Downloadable.Api.ProductRepositoryTest.testUpdateDownloadableProductSamplesWithNewFile Magento.Downloadable.Api.ProductRepositoryTest.testUpdateDownloadableProductLinks Magento.Downloadable.Api.ProductRepositoryTest.testUpdateDownloadableProductLinksWithNewFile Magento.Downloadable.Api.ProductRepositoryTest.testUpdateDownloadableProductSamples Magento.Downloadable.Api.ProductRepositoryTest.testUpdateDownloadableProductSamplesWithNewFile

Additionally, several functional tests are failing on this Pull Request, some of them you may see in the Travis CI job linked to this PR.

Could you please take a look at these test failures and maybe try to align the solution with the one described by @mslabko in the comments above?

Thanks for collaboration

[mslabko]

Here is a commit from 2.2-develop that corresponds to the first recommendation from my previous comment:
daef4b4#diff-d31bcdab992b8a290ce2d9ae7ed5bb62R488

[jameshalsall]

I'm happy to fix the broken scenarios, but can you point me to some documentation that explains how to run the functional test suite for this repository on my local?

[ishakhsuvarov]

Hi @jameshalsall
Here is the information on the api-functional tests: https://devdocs.magento.com/guides/v2.3/get-started/web-api-functional-testing.html

Also, please take a look at the comment above by @mslabko: It would be best to align the solution with the one existing in 2.2-develop

Thanks

[ishakhsuvarov]

Hi @jameshalsall
I am closing this PR now due to inactivity. Additionally, @mslabko and his team are going to implement this in a slightly different way, so you may expect the problem to be fixed.

Thank you