Skip to content

Conversation

@tianchen92
Copy link
Contributor

Related to ARROW-5834.
Apply new hash map in DictionaryEncoder to make it work.
Meanwhile provide benchmark for DictionaryEncoder:

DictionaryEncoderBenchmarks#testEncode:
Before: 5 430860.989 ± 14157.675 ns/op
After: 5 415703.943 ± 9258.049 ns/op

@tianchen92 tianchen92 closed this Jul 3, 2019
@tianchen92 tianchen92 reopened this Jul 3, 2019
@codecov-io
Copy link

Codecov Report

Merging #4786 into master will increase coverage by 2.18%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #4786      +/-   ##
=========================================
+ Coverage   87.41%   89.6%   +2.18%     
=========================================
  Files         996     661     -335     
  Lines      139489   96300   -43189     
  Branches     1418       0    -1418     
=========================================
- Hits       121938   86291   -35647     
+ Misses      17189   10009    -7180     
+ Partials      362       0     -362
Impacted Files Coverage Δ
cpp/src/plasma/common.cc 76.19% <0%> (-12.28%) ⬇️
python/pyarrow/error.pxi 51.21% <0%> (-1.85%) ⬇️
cpp/src/arrow/status-test.cc 98.3% <0%> (-1.7%) ⬇️
cpp/src/arrow/status.h 95.5% <0%> (-0.38%) ⬇️
cpp/src/arrow/python/serialize.cc 89.61% <0%> (-0.33%) ⬇️
cpp/src/plasma/client.cc 97.1% <0%> (ø) ⬆️
python/pyarrow/__init__.py 40.54% <0%> (ø) ⬆️
cpp/src/plasma/common.h 100% <0%> (ø) ⬆️
cpp/src/plasma/protocol.cc 93.76% <0%> (ø) ⬆️
cpp/src/arrow/python/common.h 98.8% <0%> (ø) ⬆️
... and 347 more

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 ecbe116...2818cbe. Read the comment docs.

@emkornfield
Copy link
Contributor

LGTM, @tianchen92 can you rebase, i think CI failures should go away.

@tianchen92
Copy link
Contributor Author

LGTM, @tianchen92 can you rebase, i think CI failures should go away.

I just rebased yesterday, should I rebase again or just reopen this PR?

@tianchen92 tianchen92 closed this Jul 5, 2019
@tianchen92 tianchen92 reopened this Jul 5, 2019
@emkornfield
Copy link
Contributor

+1 LGTM, CI failure is due to rust (unrelated)

kszucs pushed a commit that referenced this pull request Jul 22, 2019
Related to [ARROW-5834](https://issues.apache.org/jira/browse/ARROW-5834).
Apply new hash map in DictionaryEncoder to make it work.
Meanwhile provide benchmark for DictionaryEncoder:

DictionaryEncoderBenchmarks#testEncode:
Before: 5  430860.989 ± 14157.675  ns/op
After:   5  415703.943 ± 9258.049  ns/op

Author: tianchen <niki.lj@alibaba-inc.com>

Closes #4786 from tianchen92/ARROW-5834 and squashes the following commits:

dce8b6a <tianchen> fix
43f7695 <tianchen> Apply new hash map in DictionaryEncoder
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
Related to [ARROW-5834](https://issues.apache.org/jira/browse/ARROW-5834).
Apply new hash map in DictionaryEncoder to make it work.
Meanwhile provide benchmark for DictionaryEncoder:

DictionaryEncoderBenchmarks#testEncode:
Before: 5  430860.989 ± 14157.675  ns/op
After:   5  415703.943 ± 9258.049  ns/op

Author: tianchen <niki.lj@alibaba-inc.com>

Closes apache#4786 from tianchen92/ARROW-5834 and squashes the following commits:

dce8b6a <tianchen> fix
43f7695 <tianchen> Apply new hash map in DictionaryEncoder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants