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.

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

Array# support #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

dredozubov
Copy link
Contributor

@dredozubov dredozubov commented Aug 25, 2017

An exact number of fields is statically known, so there's no other reason that performance to choose SmallArray# over Array#. The issue with SmallArray# is an implicit maximum bound on the number of fields(<=128). Happily, it's possible to use multiple backends for the records of different size. Tests pass, but I want to add some new ones for the huge records specifically. Benchmarking won't hurt either.


This change is Reviewable

@dredozubov
Copy link
Contributor Author

Also I should note it's a breaking change. This will force some new constraints on the user, but I consider it a lesser evil.

@LeanderK
Copy link

LeanderK commented Oct 6, 2017

I have actually a use case for this. I am working on a DataFrames library and will probably choose superrrecord for as a backend for some parts. Having CSVs with > 128 columns is not super uncommon.

@agrafix
Copy link
Owner

agrafix commented Oct 6, 2017

We will want to support this, but at the moment there are some problems with larger records that cause massive compile time slow downs. We have to investigate those first before we can drill down on this.

@LeanderK
Copy link

LeanderK commented Oct 6, 2017

okay, no rush. It's not even close to being finished.

@dredozubov
Copy link
Contributor Author

dredozubov commented Nov 22, 2017

I've created an issue to dig into the compilation time blow-up, in a sense it blocks this PR(but maybe it isn't?) #12

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