Cstrchunk

[avati]

String based chunk support. Set contains two patches

1. Initial patch to create a string typed chunk
2. Additional patch from @bghill which has bug fixes to the first patch and wider integration with the parser etc.

[bghill]

The only catch with putting this in the main h2o-dev stream is lines 666-670 of ParsedData2.java. If the Enum maps max out, this code spills over to using strings instead of inserting NA's (the old behavior). Since string/enum unification between chunks isn't included in this patch, if anything tests that behavior it will fail. If that isn't an issue, then merge away.

Commenting out the above mentioned lines is handy for testing. All items are then read in as strings, and basic string vector functionality can be tested. Putting them back in, lets you see the standard enum behavior again (minus overflow NAs).

I hope to finish the string/enum unification tonight.

[cliffclick]

If you're that close, I might wait on Anand's pull & take 'em all.

fyi, we've been hacking the "gradle test" and "make check" paths - both
of 'em - and there are some pre-existing known failures in h2o-dev. See
if you can get your tests to run in isolation, but I'll look as soon as
something's available.

Cliff

On 7/7/2014 5:26 PM, Brandon wrote:

The only catch with putting this in the main h2o-dev stream is lines
666-670 of ParsedData2.java. If the Enum maps max out, this code
spills over to using strings instead of inserting NA's (the old
behavior). Since string/enum unification between chunks isn't included
in this patch, if anything tests that behavior it will fail. If that
isn't an issue, then merge away.

Commenting out the above mentioned lines is handy for testing. All
items are then read in as strings, and basic string vector
functionality can be tested. Putting them back in, lets you see the
standard enum behavior again (minus overflow NAs).

I hope to finish the string/enum unification tonight.

—
Reply to this email directly or view it on GitHub
#4 (comment).

[cliffclick]

Tomas - I'm poking at work between setting up for this trip.
Can you take the lead on pulling Anand's changes?
If Brandon's are ready, please review & pull them also.

We can handle the H2O1 integration later (if ever; lets work out the
bugs in h2o-dev, then we can look at how clean (or not) it goes into H2O1)

Thanks
Cliff

On 7/7/2014 5:26 PM, Brandon wrote:

The only catch with putting this in the main h2o-dev stream is lines
666-670 of ParsedData2.java. If the Enum maps max out, this code
spills over to using strings instead of inserting NA's (the old
behavior). Since string/enum unification between chunks isn't included
in this patch, if anything tests that behavior it will fail. If that
isn't an issue, then merge away.

Commenting out the above mentioned lines is handy for testing. All
items are then read in as strings, and basic string vector
functionality can be tested. Putting them back in, lets you see the
standard enum behavior again (minus overflow NAs).

I hope to finish the string/enum unification tonight.

—
Reply to this email directly or view it on GitHub
#4 (comment).

[tomasnykodym]

Ok I merged it in but I have two comments:

a) the chunk is not supported directly by the serialized bytes as other chunks are (uses two separate arrays internally and both are allocated/copied during read() ) so it will double the required memory.

b) do we want the interface to return String (need to copy the memory for each accessed row) or should we use the ValueString as is used in the parser to avoid memcopy/allocation?

[avati]

The interface was driven by the first requirement (Mahout), in which case
String was needed (to create a Scala Array of Strings for each Chunk).
Unfortunately it is expensive.

On Tue, Jul 8, 2014 at 3:53 PM, tomasnykodym notifications@github.com
wrote:

Ok I merged it in but I have two comments:

a) the chunk is not supported directly by the serialized bytes as other
chunks are (uses two separate arrays internally and both are
allocated/copied during read() ) so it will double the required memory.

b) do we want the interface to return String (need to copy the memory for
each accessed row) or should we use the ValueString as is used in the
parser to avoid memcopy/allocation?

—
Reply to this email directly or view it on GitHub
#4 (comment).

[cliffclick]

Good catch. Yes, that's a fail.
It's part of the complex/subtle contract on Chunks - no separate
ser/deser step, always only raw bytes - to avoid the massive
copy/allocation costs in moving data around.

So yes, please, flip CStrChunk to a pile-o-bytes, and have the interface
take & return ValueStrings instead.
We can have a 2nd convenience interface that returns a newly allocated
ValueString, but the bulk/efficient one will just fill in a pre-existing
ValueString - allowing you to iterate over all strings without doing
any allocation. Ability to do this is a major design requirement
(controls GC costs).

Brandon, are you up to doing this? Can you coordinate w/Tomas & Anand
(as I'm only sorta on-grid now)

Thanks,
Cliff

On 7/8/2014 3:53 PM, tomasnykodym wrote:

Ok I merged it in but I have two comments:

a) the chunk is not supported directly by the serialized bytes as
other chunks are (uses two separate arrays internally and both are
allocated/copied during read() ) so it will double the required memory.

b) do we want the interface to return String (need to copy the memory
for each accessed row) or should we use the ValueString as is used in
the parser to avoid memcopy/allocation?

—
Reply to this email directly or view it on GitHub
#4 (comment).

[cliffclick]

Yes, But No. Internally we'll use ValueString.
We can add an expensive convenience interface, but always supply the
"main" interface as a no-allocation path. It might not be the "main"
interface in terms of places where it's used... but it must be possible
to do so in all the hot paths. Imagine, e.g., "grep" for an address.
Complete fail (probably 10x slower) to allocate all strings. And
storing as-strings blows the all-big-data-is-in-big-primitive-arrays
invariant, leading to GC costs.

We'll look at a double-storage option (both strings & bytes) if String
ops become popular.
Cleaner can drop the Strings on demand, but FullGC will run slower.
Internally we'll use ValueString, as it's only slightly more headache
and a lot faster.

Cliff

On 7/8/2014 4:07 PM, Anand Avati wrote:

The interface was driven by the first requirement (Mahout), in which case
String was needed (to create a Scala Array of Strings for each Chunk).
Unfortunately it is expensive.

On Tue, Jul 8, 2014 at 3:53 PM, tomasnykodym notifications@github.com
wrote:

Ok I merged it in but I have two comments:

a) the chunk is not supported directly by the serialized bytes as other
chunks are (uses two separate arrays internally and both are
allocated/copied during read() ) so it will double the required memory.

b) do we want the interface to return String (need to copy the
memory for
each accessed row) or should we use the ValueString as is used in the
parser to avoid memcopy/allocation?

—
Reply to this email directly or view it on GitHub
#4 (comment).

—
Reply to this email directly or view it on GitHub
#4 (comment).