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

C# refactoring, memory leak fixes, general goodness,... #863

Merged
merged 186 commits into from Dec 21, 2015

Conversation

eisber
Copy link
Collaborator

@eisber eisber commented Nov 23, 2015

fixed Runtime library mismatch between zlib, libvw, vw.exe, VowpalWabbitCore.dll (CLR),... by using zlib/boost nuget provided msbuild targets
included Visual Leak Detector for memory leak detection on windows
refactored C# API to allow users to dynamically constructor serializers based on alternate descriptions (not just on static annotations)
string marshalling is compatible to command line (either escaping or splitting)
schema based pre-hashing: if hash can be determine from schema it's only generated once and re-used for each example.
added type extension API for marshalling
allow user to generate native and string examples in parallel in both debug and release
keep marshalling expression tree for debugging
refactored marshalling expression tree generation to improve readability
added sweeping helper
improved C# label parsing extensibility
added assembly signing
fixed memory leaks in C# usage of VW
fixed model hashing/reload interaction
fixed handling of empty line examples within set of action dependent features
fixed order issue when predicting ADF examples containing empty action dependent features
fixed default namespace incompatibility (space vs. 0)
improved RunTests to C# test wrapping (detects inter-test dependencies and input files)
unit tests are run in test/ folder, thus no need copy all input files
added user-supplied model id support

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

Pushed.

From: John <notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit <reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 11:17
To: JohnLangford/vowpal_wabbit <vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes, general goodness,... (#863)

Done.

-John

On 12/21/2015 11:08 AM, Markus Cozowicz wrote:

Can you run

find -regextype posix-extended -regex '..(cc|h|cpp)' | xargs eclint fix
find -regextype posix-extended -regex '.
.(cc|h|cpp)' | grep -v
vw_clr | xargs astyle -s2 --style=horstmann --lineend=linux
--keep-one-line-blocks --keep-one-line-statements

on vowpal_wabbit/vowpalwabbit/* and push?

Otherwise it’s super hard to see if I miss any real changes…

Markus

From: John <notifications@github.commailto:notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit
<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 10:55
To: JohnLangford/vowpal_wabbit
<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

It should not be? I'm pretty sure the bfgs.cc code is unchanged since
the last version release.

-John

On 12/21/2015 10:52 AM, Markus Cozowicz wrote:

I just reran Louies formatting commands and still see lots of change.
Can this be right?

From: John
<notifications@github.commailto:notifications@github.commailto:notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit

<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 09:30
To: JohnLangford/vowpal_wabbit

<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz
<marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

Why is struct bfgs reformatted?


Reply to this email directly or view it on

GitHubhttps://github.com//pull/863#issuecomment-166317657.


Reply to this email directly or view it on GitHub

#863 (comment).


Reply to this email directly or view it on
GitHubhttps://github.com//pull/863#issuecomment-166338748.


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


Reply to this email directly or view it on GitHubhttps://github.com//pull/863#issuecomment-166347265.

@JohnLangford
Copy link
Member

It says "

    This branch has conflicts that must be resolved"

Fix?

-John

On 12/21/2015 11:43 AM, Markus Cozowicz wrote:

Pushed.

From: John <notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit
<reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 11:17
To: JohnLangford/vowpal_wabbit
<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

Done.

-John

On 12/21/2015 11:08 AM, Markus Cozowicz wrote:

Can you run

find -regextype posix-extended -regex '..(cc|h|cpp)' | xargs
eclint fix
find -regextype posix-extended -regex '.
.(cc|h|cpp)' | grep -v
vw_clr | xargs astyle -s2 --style=horstmann --lineend=linux
--keep-one-line-blocks --keep-one-line-statements

on vowpal_wabbit/vowpalwabbit/* and push?

Otherwise it’s super hard to see if I miss any real changes…

Markus

From: John
<notifications@github.commailto:notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit

<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 10:55
To: JohnLangford/vowpal_wabbit

<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz
<marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

It should not be? I'm pretty sure the bfgs.cc code is unchanged since
the last version release.

-John

On 12/21/2015 10:52 AM, Markus Cozowicz wrote:

I just reran Louies formatting commands and still see lots of change.
Can this be right?

From: John

<notifications@github.commailto:notifications@github.commailto:notifications@github.commailto:notifications@github.com>

Reply-To: JohnLangford/vowpal_wabbit

<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>

Date: Monday, December 21, 2015 at 09:30
To: JohnLangford/vowpal_wabbit

<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>

Cc: Markus Cozowicz

<marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>

Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

Why is struct bfgs reformatted?


Reply to this email directly or view it on

GitHubhttps://github.com//pull/863#issuecomment-166317657.


Reply to this email directly or view it on GitHub

#863 (comment).


Reply to this email directly or view it on

GitHubhttps://github.com//pull/863#issuecomment-166338748.


Reply to this email directly or view it on GitHub

#863 (comment).


Reply to this email directly or view it on
GitHubhttps://github.com//pull/863#issuecomment-166347265.


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

@JohnLangford
Copy link
Member

Can all the cs_/ go into cs/?

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

That’s what I’m doing right now. Need to go through all the cs_proj files.

From: John <notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit <reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 11:55
To: JohnLangford/vowpal_wabbit <vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes, general goodness,... (#863)

Can all the cs_/ go into cs/?


Reply to this email directly or view it on GitHubhttps://github.com//pull/863#issuecomment-166358241.

@JohnLangford
Copy link
Member

Cool.

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

My tests are crashing after upstream merge…

From: John <notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit <reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 11:50
To: JohnLangford/vowpal_wabbit <vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes, general goodness,... (#863)

It says "

This branch has conflicts that must be resolved"

Fix?

-John

On 12/21/2015 11:43 AM, Markus Cozowicz wrote:

Pushed.

From: John <notifications@github.commailto:notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit
<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 11:17
To: JohnLangford/vowpal_wabbit
<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

Done.

-John

On 12/21/2015 11:08 AM, Markus Cozowicz wrote:

Can you run

find -regextype posix-extended -regex '..(cc|h|cpp)' | xargs
eclint fix
find -regextype posix-extended -regex '.
.(cc|h|cpp)' | grep -v
vw_clr | xargs astyle -s2 --style=horstmann --lineend=linux
--keep-one-line-blocks --keep-one-line-statements

on vowpal_wabbit/vowpalwabbit/* and push?

Otherwise it’s super hard to see if I miss any real changes…

Markus

From: John
<notifications@github.commailto:notifications@github.commailto:notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit

<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 10:55
To: JohnLangford/vowpal_wabbit

<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz
<marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

It should not be? I'm pretty sure the bfgs.cc code is unchanged since
the last version release.

-John

On 12/21/2015 10:52 AM, Markus Cozowicz wrote:

I just reran Louies formatting commands and still see lots of change.
Can this be right?

From: John

<notifications@github.commailto:notifications@github.commailto:notifications@github.commailto:notifications@github.commailto:notifications@github.com>

Reply-To: JohnLangford/vowpal_wabbit

<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>

Date: Monday, December 21, 2015 at 09:30
To: JohnLangford/vowpal_wabbit

<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>

Cc: Markus Cozowicz

<marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>

Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

Why is struct bfgs reformatted?


Reply to this email directly or view it on

GitHubhttps://github.com//pull/863#issuecomment-166317657.


Reply to this email directly or view it on GitHub

#863 (comment).


Reply to this email directly or view it on

GitHubhttps://github.com//pull/863#issuecomment-166338748.


Reply to this email directly or view it on GitHub

#863 (comment).


Reply to this email directly or view it on
GitHubhttps://github.com//pull/863#issuecomment-166347265.


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


Reply to this email directly or view it on GitHubhttps://github.com//pull/863#issuecomment-166356773.

@JohnLangford
Copy link
Member

Unclear what's up? The master branch is in the same state as before as
far as I know?

-John

On 12/21/2015 12:22 PM, Markus Cozowicz wrote:

My tests are crashing after upstream merge…

From: John <notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit
<reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 11:50
To: JohnLangford/vowpal_wabbit
<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

It says "

This branch has conflicts that must be resolved"

Fix?

-John

On 12/21/2015 11:43 AM, Markus Cozowicz wrote:

Pushed.

From: John
<notifications@github.commailto:notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit

<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 11:17
To: JohnLangford/vowpal_wabbit

<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz
<marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

Done.

-John

On 12/21/2015 11:08 AM, Markus Cozowicz wrote:

Can you run

find -regextype posix-extended -regex '..(cc|h|cpp)' | xargs
eclint fix
find -regextype posix-extended -regex '.
.(cc|h|cpp)' | grep -v
vw_clr | xargs astyle -s2 --style=horstmann --lineend=linux
--keep-one-line-blocks --keep-one-line-statements

on vowpal_wabbit/vowpalwabbit/* and push?

Otherwise it’s super hard to see if I miss any real changes…

Markus

From: John

<notifications@github.commailto:notifications@github.commailto:notifications@github.commailto:notifications@github.com>

Reply-To: JohnLangford/vowpal_wabbit

<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>

Date: Monday, December 21, 2015 at 10:55
To: JohnLangford/vowpal_wabbit

<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>

Cc: Markus Cozowicz

<marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>

Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

It should not be? I'm pretty sure the bfgs.cc code is unchanged since
the last version release.

-John

On 12/21/2015 10:52 AM, Markus Cozowicz wrote:

I just reran Louies formatting commands and still see lots of
change.
Can this be right?

From: John

<notifications@github.commailto:notifications@github.commailto:notifications@github.commailto:notifications@github.commailto:notifications@github.com>

Reply-To: JohnLangford/vowpal_wabbit

<reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.commailto:reply@reply.github.com>

Date: Monday, December 21, 2015 at 09:30
To: JohnLangford/vowpal_wabbit

<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>

Cc: Markus Cozowicz

<marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.commailto:marcozo@microsoft.com>

Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

Why is struct bfgs reformatted?


Reply to this email directly or view it on

GitHubhttps://github.com//pull/863#issuecomment-166317657.


Reply to this email directly or view it on GitHub

#863 (comment).


Reply to this email directly or view it on

GitHubhttps://github.com//pull/863#issuecomment-166338748.


Reply to this email directly or view it on GitHub

#863 (comment).


Reply to this email directly or view it on

GitHubhttps://github.com//pull/863#issuecomment-166347265.


Reply to this email directly or view it on GitHub

#863 (comment).


Reply to this email directly or view it on
GitHubhttps://github.com//pull/863#issuecomment-166356773.


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

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

I believe there are more changes on the master compared to when I last sync’d. I moved all C# into cs/, but my VS Test environment crashes and I can’t properly debug.
Also changes in parse_regressor.cc (buff/buff2 allocation) introduced a memory leak – fixed, but it’s the usual ugliness (double free in catch(…) and outside)

Markus

@JohnLangford
Copy link
Member

What disables debugging? For any file, you can get the changes between
two checkins using: git diff .

-John

On 12/21/2015 01:09 PM, Markus Cozowicz wrote:

I believe there are more changes on the master compared to when I last
sync’d. I moved all C# into cs/, but my VS Test environment crashes
and I can’t properly debug.
Also changes in parse_regressor.cc (buff/buff2 allocation) introduced
a memory leak – fixed, but it’s the usual ugliness (double free in
catch(…) and outside)

Markus


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

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

Already looked at the changes and didn’t see anything suspicious. Investigating…

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

My diff output below. From what I can find on the web, git only stores files as 644 or 755. I actually don’t understand why it should have been something else… I think 644 is the right mode – non-executable as it should be for a model file.

mwtds@ubuntu:~/vw.eisber/vowpal_wabbit/test/model-sets$ git diff upstream/master 7.10.2_corrupted.model
diff --git a/test/model-sets/7.10.2_corrupted.model b/test/model-sets/7.10.2_corrupted.model
new file mode 100644
index 0000000..05e9f4d

From: John [mailto:notifications@github.com]
Sent: Monday, December 21, 2015 9:28 AM
To: JohnLangford/vowpal_wabbit vowpal_wabbit@noreply.github.com
Cc: Markus Cozowicz marcozo@microsoft.com
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes, general goodness,... (#863)

why are attributes changed on test/model-sets/7.10.2_corrupted.model?
On utl/vw-experiment ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/863#issuecomment-166316997.

@JohnLangford
Copy link
Member

644 does seem like the right mode. I'm just unclear on why it is
changing at all.

-John

On 12/21/2015 02:05 PM, Markus Cozowicz wrote:

My diff output below. From what I can find on the web, git only stores
files as 644 or 755. I actually don’t understand why it should have
been something else… I think 644 is the right mode – non-executable as
it should be for a model file.

mwtds@ubuntu:~/vw.eisber/vowpal_wabbit/test/model-sets$ git diff
upstream/master 7.10.2_corrupted.model
diff --git a/test/model-sets/7.10.2_corrupted.model
b/test/model-sets/7.10.2_corrupted.model
new file mode 100644
index 0000000..05e9f4d

From: John [mailto:notifications@github.com]
Sent: Monday, December 21, 2015 9:28 AM
To: JohnLangford/vowpal_wabbit vowpal_wabbit@noreply.github.com
Cc: Markus Cozowicz marcozo@microsoft.com
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

why are attributes changed on test/model-sets/7.10.2_corrupted.model?
On utl/vw-experiment ?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/863#issuecomment-166316997.


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

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

From the diff it looks like there was no mode set all. I’m not sure if one can return to that state…

From: John <notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit <reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 14:40
To: JohnLangford/vowpal_wabbit <vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes, general goodness,... (#863)

644 does seem like the right mode. I'm just unclear on why it is
changing at all.

-John

On 12/21/2015 02:05 PM, Markus Cozowicz wrote:

My diff output below. From what I can find on the web, git only stores
files as 644 or 755. I actually don’t understand why it should have
been something else… I think 644 is the right mode – non-executable as
it should be for a model file.

mwtds@ubuntu:~/vw.eisber/vowpal_wabbit/test/model-sets$ git diff
upstream/master 7.10.2_corrupted.model
diff --git a/test/model-sets/7.10.2_corrupted.model
b/test/model-sets/7.10.2_corrupted.model
new file mode 100644
index 0000000..05e9f4d

From: John [mailto:notifications@github.com]
Sent: Monday, December 21, 2015 9:28 AM
To: JohnLangford/vowpal_wabbit <vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

why are attributes changed on test/model-sets/7.10.2_corrupted.model?
On utl/vw-experiment ?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/863#issuecomment-166316997.


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


Reply to this email directly or view it on GitHubhttps://github.com//pull/863#issuecomment-166399727.

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

Found 2 issues. Visual Leak Detector somehow sneaked into Debug builds.

I found the offending commit: 923bbe6 from Will Newton.
%z is not supported on Windows.

Changing to %llu http://stackoverflow.com/questions/6655410/why-doesnt-zd-printf-format-work-in-vs2010

Still one to go…

@JohnLangford
Copy link
Member

Cool.

-John

On 12/21/2015 04:03 PM, Markus Cozowicz wrote:

Found 2 issues. Visual Leak Detector somehow sneaked into Debug builds.

I found the offending commit: 923bbe6 from Will Newton.
%z is not supported on Windows.

Changing to %llu
http://stackoverflow.com/questions/6655410/why-doesnt-zd-printf-format-work-in-vs2010

Still one to go…


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

fixed model loading code for corrupted models
fixed %z specifier (not supported on Windows)
@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

Pushed.

Found the 2nd offender too. The extension for dynamic memory when reading model introduced for the Java API didn’t care for corrupted models.
The Diff got quite a big larger, as I moved all projects into cs/.

I’ll have to wait for Visual Leak Detector nuget to be build, minor change to make sure there isn’t an unwanted dependency onto VLD in Debug.

Markus

@JohnLangford
Copy link
Member

Checks failing still?

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

Cs/ move around aftermath…

@eisber
Copy link
Collaborator Author

eisber commented Dec 21, 2015

fixed

From: John <notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit <reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 17:21
To: JohnLangford/vowpal_wabbit <vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes, general goodness,... (#863)

Checks failing still?


Reply to this email directly or view it on GitHubhttps://github.com//pull/863#issuecomment-166439771.

JohnLangford added a commit that referenced this pull request Dec 21, 2015
C# refactoring, memory leak fixes, general goodness,...
@JohnLangford JohnLangford merged commit 537f18e into VowpalWabbit:master Dec 21, 2015
@JohnLangford
Copy link
Member

Merged at last!

All, this is a major update to the C# .net library interface, plus many
minor robustness and memory leak fixes.

-John

On 12/21/2015 05:56 PM, Markus Cozowicz wrote:

fixed

From: John <notifications@github.commailto:notifications@github.com>
Reply-To: JohnLangford/vowpal_wabbit
<reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, December 21, 2015 at 17:21
To: JohnLangford/vowpal_wabbit
<vowpal_wabbit@noreply.github.commailto:vowpal_wabbit@noreply.github.com>
Cc: Markus Cozowicz <marcozo@microsoft.commailto:marcozo@microsoft.com>
Subject: Re: [vowpal_wabbit] C# refactoring, memory leak fixes,
general goodness,... (#863)

Checks failing still?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/863#issuecomment-166439771.


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

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.

None yet

5 participants