8000 fix: fix docarray iter by JoanFM · Pull Request #2604 · jina-ai/serve · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: fix docarray iter #2604

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
Merged
merged 2 commits into from
Jun 10, 2021
Merged

fix: fix docarray iter #2604

merged 2 commits into from
Jun 10, 2021

Conversation

JoanFM
Copy link
Contributor
@JoanFM JoanFM commented Jun 9, 2021 •
edited by davidbp
Loading

I did some benchmarking on the updated __iter__ method and it does not seem to degrade at all.

I used the following test

n_samples = 5

def iterate_over_d(darr):
    i = 0
    for d in darr:
        i+=1

for i in range(5):
    n_samples = n_samples *10
    darr = DocumentArray([jina_pb2.DocumentProto()]*n_samples)
    print(f'n_samples={n_samples}')
    %timeit iterate_over_d(darr)

This branch results

n_samples=50
393 µs ± 22.1 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
n_samples=500
3.74 ms ± 60.1 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
n_samples=5000
36.7 ms ± 285 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
n_samples=50000
369 ms ± 4.13 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
n_samples=500000
3.74 s ± 43.1 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

Master branch results

n_samples=50
369 µs ± 4.39 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
n_samples=500
3.68 ms ± 59.1 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
n_samples=5000
37.7 ms ± 686 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
n_samples=50000
381 ms ± 15.7 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
n_samples=500000
3.74 s ± 72.1 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

@jina-bot jina-bot added size/S area/core This issue/PR affects the core codebase area/testing This issue/PR affects testing component/type labels Jun 9, 2021
@JoanFM JoanFM linked an issue Jun 9, 2021 that may be closed by this pull request
DocumentArray of GraphDocument elements gets a Document when iterating #2603
Closed
@JoanFM JoanFM mentioned this pull request Jun 9, 2021
Closed
@codecov
Copy link
codecov bot commented Jun 9, 2021 •
edited
Loading

Codecov Report

Merging #2604 (4be4505) into master (7705fbb) will increase coverage by 2.26%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2604      +/-   ##
==========================================
+ Coverage   85.10%   87.36%   +2.26%     
==========================================
  Files         153      153              
  Lines        9613     9618       +5     
==========================================
+ Hits         8181     8403     +222     
+ Misses       1432     1215     -217
Flag Coverage Δ
daemon 46.56% <0.00%> (-0.03%) ⬇️
jina 87.44% <100.00%> (+2.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jina/types/arrays/document.py 84.73% <100.00%> (+0.41%) ⬆️
jina/peapods/pods/compound.py 90.97% <0.00%> (+1.50%) ⬆️
jina/parsers/helloworld.py 96.22% <0.00%> (+1.88%) ⬆️
jina/peapods/peas/__init__.py 96.77% <0.00%> (+2.41%) ⬆️
jina/helloworld/fashion/app.py 83.33% <0.00%> (+83.33%) ⬆️
jina/helloworld/fashion/helper.py 90.42% <0.00%> (+90.42%) ⬆️
jina/helloworld/fashion/my_executors.py 94.64% <0.00%> (+94.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7705fbb...4be4505. Read the comment docs.

@JoanFM JoanFM marked this pull request as ready for review June 10, 2021 07:21
@JoanFM JoanFM requested a review from a team as a code owner June 10, 2021 07:21
@JoanFM JoanFM requested review from Roshanjossey and slettner June 10, 2021 07:21
@davidbp
Copy link
Contributor
davidbp commented Jun 10, 2021 •
edited
Loading

LGTM!
I would conclude that the results shown suggest the change does not imply any performance problem

@JoanFM JoanFM merged commit 076dc92 into master Jun 10, 2021
@JoanFM JoanFM deleted the docarray-iter branch June 10, 2021 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbp davidbp davidbp approved these changes

@Roshanjossey Roshanjossey Awaiting requested review from Roshanjossey

@slettner slettner Awaiting requested review from slettner

Assignees
No one assigned
Labels
area/core This issue/PR affects the core codebase area/testing This issue/PR affects testing component/type size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DocumentArray of GraphDocument elements gets a Document when iterating
3 participants
@JoanFM @davidbp @jina-bot
0