Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimize common package functions for improved performance #187

Merged
merged 7 commits into from
May 18, 2023
Merged

Optimize common package functions for improved performance #187

merged 7 commits into from
May 18, 2023

Commits on May 18, 2023

  1. optimisation: Improve performance of UnMarshallObjectKey 

    This commit optimizes the UnMarshallObjectKey function by avoiding unnecessary allocations and using a simple string split operation instead of manual string parsing. This results in performance improvements when processing large volumes of Kubernetes objects. Additionally, the function now validates input strings more accurately to prevent errors when working with invalid object keys.
    @art-tapin
    art-tapin committed May 18, 2023
    Configuration menu
    Copy the full SHA
    b5fe67b View commit details
    Browse the repository at this point in the history

  2. optimisation: Improve performance of UnMarshallLimitNamespace 

    This commit optimizes the UnMarshallLimitNamespace function to improve its runtime performance. The following changes were made:

- Used strings.IndexByte instead of strings.Split to search for the index of the '#' delimiter character, reducing the number of iterations and memory allocation overhead.
- Extracted the domain directly from the input string instead of creating a temporary slice, reducing the number of string allocations.
- Passed only the first half of the input string to UnMarshallObjectKey function, avoiding the creation of unnecessary intermediate strings.
    @art-tapin
    art-tapin committed May 18, 2023
    Configuration menu
    Copy the full SHA
    b96d2c3 View commit details
    Browse the repository at this point in the history

  3. optimisation: improve performance of HostnamesToStrings (#167) 

    - Pre-allocated output slice using make function to reduce append overhead
    @art-tapin
    art-tapin committed May 18, 2023
    Configuration menu
    Copy the full SHA
    b1bb6e1 View commit details
    Browse the repository at this point in the history

  4. refactor: Replace errors.New(...) with fmt.Errorf(...)

    @art-tapin
    art-tapin committed May 18, 2023
    Configuration menu
    Copy the full SHA
    13436ea View commit details
    Browse the repository at this point in the history

  5. refactor: Simplify if-condition in UnMarshallObjectKey to handle empt… 

    …y namespace and name values

+ refactor the error message
    @art-tapin
    art-tapin committed May 18, 2023
    Configuration menu
    Copy the full SHA
    78621a5 View commit details
    Browse the repository at this point in the history

  6. fix: Use IndexRune instead of IndexByte to handle non-ascii character… 

    …s in UnMarshallLimitNamespace and UnMarshallObjectKey

Discussion: #187 (comment)

Kudos to Eguzki
    @art-tapin
    art-tapin committed May 18, 2023
    Configuration menu
    Copy the full SHA
    532cf2e View commit details
    Browse the repository at this point in the history

  7. [test] Unit-tests for common/common.go (part 3 of 3) (#186) 

    * test: Add unit-tests for UnMarshallLimitNamespace (#167)

* test: Add two FAILING unit-tests for UnMarshallLimitNamespace (#167)

The two failing test cases include:
- when namespace has no gateway name then return an error
- when namespace has no domain name then return an error

In these test-cases we leave the separator (either "/" or "#"), which causes the function to return an error when the gateway name or domain name is missing. In the last two test cases:
- when namespace only has gateway name (missing 'namespace/') and domain then return an error
- when namespace only has namespace name (missing '/gwName') and domain then return an error
the separator (/) is missing along with the namespace or gateway names, causing the function to return the correct error message.

* refactor: Add comment to UnMarshallLimitNamespace (#167)

* test: Add unit-tests for MarshallNamespace (#167)

* test: Add unit-tests for UnMarshallObjectKey (#167)

* test: Add unit-tests for HostnamesToStrings (#167)

* [test] Optimizations, improvements, and unit tests for common/common.go (part 2 of 3) (#182)

* test: Add unit-tests for Contains (#167)

* refactor: Add comment to Contains (#167)

* test: Add unit-tests for Map (#167)

* refactor: Add comment to Map (#167)

* refactor: Add names to Map test cases (#167)

* test: Add unit-tests for SliceCopy (#167)

* refactor: Add comment to SliceCopy (#167)

* test: Add unit-tests for ReverseSlice (#167)

* refactor: Add comment to ReverseSlice (#167)

* test: Add unit-tests for MergeMapStringString (#167)

* refactor: Add missing import 'reflect'

* fix: Ensure Istio gateways created in the tests are ready ('Programmed') by setting ClusterIP service type (#185)

* changelog (#184)

* test: Add two test-cases for UnMarshallObjectKey (#167)

1. when valid namespace and empty name (strKey ends with '/') then return valid ObjectKey with namespace only
2. when valid name and empty namespace (strKey starts with '/') then return valid ObjectKey with name only

Changes were discussed here: #187 (comment)

* test: Delete test case for UnMarshallLimitNamespace

The test case "when namespace has no gateway name then return an error" has been removed from the test suite for UnMarshallLimitNamespace. This test case was tightly connected to the implementation of UnMarshallObjectKey, making it impossible to properly test the desired behavior in the context of UnMarshallLimitNamespace.

* refactor: Add clarity to the test name

Describing the '/' separator as a default one we show that it is a default constant value.

---------

Co-authored-by: Guilherme Cassolato <guicassolato@gmail.com>
Co-authored-by: Eguzki Astiz Lezaun <eastizle@redhat.com>
    @art-tapin @guicassolato @eguzki
    3 people committed May 18, 2023
    Configuration menu
    Copy the full SHA
    5255cf7 View commit details
    Browse the repository at this point in the history