Skip to content

Commit 515d169

Browse files
andyclaude
andcommitted
#83 instrumentation: low-overhead by default, an A/B toggle for the fopen theory
The existing ECCE_DEBUG_MATERIAL probes did fopen()/fclose() on every atom of every frame -- real file-open/close syscalls in the render hot path, for as long as the app runs. That is enough of a timing perturbation that it may have been avoiding the very race #83 is chasing, rather than proving the material state clean: two live tries with that logging on reproduced no tint, after one had just reproduced it with logging off. Default behaviour now opens the log once and keeps the handle, flushing after each write instead of closing. Also removed the theAtom < 4 cap on the per-sphere probe, which silently never covered a 6-atom molecule's oxygen or its last hydrogen -- exactly the atoms the reported tint's colour measurements were taken from. ECCE_DEBUG_MATERIAL_HEAVY=1 restores the exact old per-call cost (one open+close syscall pair per log statement, same granularity the original probes had) so the fopen-overhead theory can be A/B tested directly instead of argued about. Live results so far (small sample, not conclusive): heavy logging has not reproduced the tint in 2 attempts; light logging reproduced it once in 3. Suggestive, not proven -- worth more attempts before trusting either way. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4HPD1xaxKpyh9gZ6hsBP2
1 parent 3557985 commit 515d169

2 files changed

Lines changed: 117 additions & 68 deletions

File tree

‎src/inv/moiv/ChemDisplay.C‎

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -688,39 +688,51 @@ ChemDisplay::GLRender(SoGLRenderAction *action)
688688
// which is why an earlier attempt at this produced nothing and
689689
// could not be told apart from the code never running.
690690
{
691+
// Opened once, not per call -- see ChemDisplaySpheres.C's
692+
// debugMaterialLog() for why (fopen/fclose in the render hot
693+
// path may itself have been masking the #83 race).
694+
// ECCE_DEBUG_MATERIAL_HEAVY=1 restores the old per-call cost
695+
// for an A/B test -- same toggle as ChemDisplaySpheres.C.
696+
static bool heavy = (getenv("ECCE_DEBUG_MATERIAL_HEAVY") != 0);
697+
static FILE *log = NULL;
698+
static bool tried = false;
691699
const char *where = getenv("ECCE_DEBUG_MATERIAL");
692-
if (where != 0) {
693-
FILE *log = fopen(where, "a");
694-
if (log != 0) {
695-
GLfloat amb[4], emi[4], dif[4], spc[4], lm[4];
696-
glGetMaterialfv(GL_FRONT, GL_AMBIENT, amb);
697-
glGetMaterialfv(GL_FRONT, GL_EMISSION, emi);
698-
glGetMaterialfv(GL_FRONT, GL_DIFFUSE, dif);
699-
glGetMaterialfv(GL_FRONT, GL_SPECULAR, spc);
700-
glGetFloatv(GL_LIGHT_MODEL_AMBIENT, lm);
701-
GLboolean cm = glIsEnabled(GL_COLOR_MATERIAL);
702-
GLint mode = 0;
703-
glGetIntegerv(GL_COLOR_MATERIAL_PARAMETER, &mode);
700+
if (heavy && where != 0) {
701+
if (log != 0) fclose(log);
702+
log = fopen(where, "a");
703+
} else if (!tried) {
704+
tried = true;
705+
if (where != 0) log = fopen(where, "a");
706+
}
707+
if (log != 0) {
708+
GLfloat amb[4], emi[4], dif[4], spc[4], lm[4];
709+
glGetMaterialfv(GL_FRONT, GL_AMBIENT, amb);
710+
glGetMaterialfv(GL_FRONT, GL_EMISSION, emi);
711+
glGetMaterialfv(GL_FRONT, GL_DIFFUSE, dif);
712+
glGetMaterialfv(GL_FRONT, GL_SPECULAR, spc);
713+
glGetFloatv(GL_LIGHT_MODEL_AMBIENT, lm);
714+
GLboolean cm = glIsEnabled(GL_COLOR_MATERIAL);
715+
GLint mode = 0;
716+
glGetIntegerv(GL_COLOR_MATERIAL_PARAMETER, &mode);
717+
fprintf(log,
718+
"MATERIAL amb %.3f %.3f %.3f | emi %.3f %.3f %.3f | "
719+
"dif %.3f %.3f %.3f | spc %.3f %.3f %.3f | "
720+
"lightmodel %.3f %.3f %.3f | colorMaterial %d mode 0x%x\n",
721+
amb[0],amb[1],amb[2], emi[0],emi[1],emi[2],
722+
dif[0],dif[1],dif[2], spc[0],spc[1],spc[2],
723+
lm[0],lm[1],lm[2], (int)cm, (unsigned)mode);
724+
for (int li = 0; li < 8; li++) {
725+
if (!glIsEnabled(GL_LIGHT0 + li)) continue;
726+
GLfloat la[4], ld[4], ls[4];
727+
glGetLightfv(GL_LIGHT0+li, GL_AMBIENT, la);
728+
glGetLightfv(GL_LIGHT0+li, GL_DIFFUSE, ld);
729+
glGetLightfv(GL_LIGHT0+li, GL_SPECULAR, ls);
704730
fprintf(log,
705-
"MATERIAL amb %.3f %.3f %.3f | emi %.3f %.3f %.3f | "
706-
"dif %.3f %.3f %.3f | spc %.3f %.3f %.3f | "
707-
"lightmodel %.3f %.3f %.3f | colorMaterial %d mode 0x%x\n",
708-
amb[0],amb[1],amb[2], emi[0],emi[1],emi[2],
709-
dif[0],dif[1],dif[2], spc[0],spc[1],spc[2],
710-
lm[0],lm[1],lm[2], (int)cm, (unsigned)mode);
711-
for (int li = 0; li < 8; li++) {
712-
if (!glIsEnabled(GL_LIGHT0 + li)) continue;
713-
GLfloat la[4], ld[4], ls[4];
714-
glGetLightfv(GL_LIGHT0+li, GL_AMBIENT, la);
715-
glGetLightfv(GL_LIGHT0+li, GL_DIFFUSE, ld);
716-
glGetLightfv(GL_LIGHT0+li, GL_SPECULAR, ls);
717-
fprintf(log,
718-
"LIGHT%d amb %.3f %.3f %.3f | dif %.3f %.3f %.3f | "
719-
"spc %.3f %.3f %.3f\n", li,
720-
la[0],la[1],la[2], ld[0],ld[1],ld[2], ls[0],ls[1],ls[2]);
721-
}
722-
fclose(log);
731+
"LIGHT%d amb %.3f %.3f %.3f | dif %.3f %.3f %.3f | "
732+
"spc %.3f %.3f %.3f\n", li,
733+
la[0],la[1],la[2], ld[0],ld[1],ld[2], ls[0],ls[1],ls[2]);
723734
}
735+
fflush(log);
724736
}
725737
}
726738

‎src/inv/moiv/ChemDisplaySpheres.C‎

Lines changed: 75 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,40 @@ static int32_t atomStart;
135135
static int32_t atomEnd;
136136
static int32_t theAtom;
137137

138+
// TEMPORARY, #83: a shared log handle for ECCE_DEBUG_MATERIAL.
139+
// The earlier probes did fopen()/fclose() on every atom of every frame --
140+
// real file-open/close syscalls in the render hot path, every frame, for
141+
// as long as the app runs. That is enough of a timing perturbation that
142+
// it may have been avoiding the very race #83 is chasing (the tint did
143+
// not reproduce across two live tries with that logging on, after
144+
// reproducing once with it off). Default here is now open-once,
145+
// never-close, flush-after-each-write.
146+
//
147+
// ECCE_DEBUG_MATERIAL_HEAVY=1 restores the OLD per-call fopen()/fclose()
148+
// cost exactly (one open+close syscall pair per log statement, same
149+
// granularity the original probes had -- once per ChemDisplay::GLRender
150+
// entry, once per renderAtomsAsSpheres entry, once per atom in the
151+
// sphere draw loop) so the two can be A/B tested directly against each
152+
// other rather than argued about.
153+
static FILE *debugMaterialLog()
154+
{
155+
static bool heavy = (getenv("ECCE_DEBUG_MATERIAL_HEAVY") != 0);
156+
static FILE *log = NULL;
157+
static bool tried = false;
158+
const char *where = getenv("ECCE_DEBUG_MATERIAL");
159+
if (where == 0) return NULL;
160+
if (heavy) {
161+
if (log != 0) fclose(log);
162+
log = fopen(where, "a");
163+
return log;
164+
}
165+
if (!tried) {
166+
tried = true;
167+
log = fopen(where, "a");
168+
}
169+
return log;
170+
}
171+
138172
#define ATOMLOOP_START(INDEX) \
139173
numAtomLoops = (INDEX).getNum(); \
140174
for (atomLoop = 0; atomLoop < numAtomLoops; atomLoop++) { \
@@ -326,15 +360,13 @@ printf ("In ChemDisplay::renderAtomsAsSpheres\n");
326360
// so a probe here runs whichever of the many copy-pasted draw
327361
// loops below is taken.
328362
{
329-
const char *where = getenv("ECCE_DEBUG_MATERIAL");
330-
if (where != 0) {
331-
FILE *log = fopen(where, "a");
332-
if (log != 0) {
333-
GLfloat amb[4], emi[4], dif[4], col[4];
334-
glGetMaterialfv(GL_FRONT, GL_AMBIENT, amb);
335-
glGetMaterialfv(GL_FRONT, GL_EMISSION, emi);
336-
glGetMaterialfv(GL_FRONT, GL_DIFFUSE, dif);
337-
glGetFloatv(GL_CURRENT_COLOR, col);
363+
FILE *log = debugMaterialLog();
364+
if (log != 0) {
365+
GLfloat amb[4], emi[4], dif[4], col[4];
366+
glGetMaterialfv(GL_FRONT, GL_AMBIENT, amb);
367+
glGetMaterialfv(GL_FRONT, GL_EMISSION, emi);
368+
glGetMaterialfv(GL_FRONT, GL_DIFFUSE, dif);
369+
glGetFloatv(GL_CURRENT_COLOR, col);
338370
GLfloat lm[4];
339371
glGetFloatv(GL_LIGHT_MODEL_AMBIENT, lm);
340372
fprintf(log, "ENTRY-LIGHTMODEL %.3f %.3f %.3f\n",
@@ -352,8 +384,14 @@ printf ("In ChemDisplay::renderAtomsAsSpheres\n");
352384
"dif %.3f %.3f %.3f\n",
353385
col[0],col[1],col[2], amb[0],amb[1],amb[2],
354386
emi[0],emi[1],emi[2], dif[0],dif[1],dif[2]);
355-
fclose(log);
356-
}
387+
// TEMPORARY, #83: is highlightSphere* ever taken with
388+
// real atoms in it, even though nothing is selected?
389+
// renderHighlight is hardcoded TRUE at every call site
390+
// (ChemDisplay.C), so the only thing gating it is
391+
// whether highlightIndex actually has entries.
392+
fprintf(log, "SPHERE-INDEX normal=%d highlight=%d\n",
393+
normalIndex.getNum(), highlightIndex.getNum());
394+
fflush(log);
357395
}
358396
}
359397

@@ -382,18 +420,15 @@ printf ("In ChemDisplay::renderAtomsAsSpheres\n");
382420
}
383421
const SbColor *atomColors = chemColor->atomColor.getValues(0);
384422
{ // TEMPORARY, #83: the colours actually handed to GL.
385-
const char *w2 = getenv("ECCE_DEBUG_MATERIAL");
386-
if (w2 != 0) {
387-
FILE *lg = fopen(w2, "a");
388-
if (lg != 0) {
389-
const int n = chemColor->atomColor.getNum();
390-
for (int ci = 0; ci < 4 && ci < n; ci++) {
391-
fprintf(lg, "ATOMCOLOR %d = %.3f %.3f %.3f\n", ci,
392-
atomColors[ci][0], atomColors[ci][1],
393-
atomColors[ci][2]);
394-
}
395-
fclose(lg);
423+
FILE *lg = debugMaterialLog();
424+
if (lg != 0) {
425+
const int n = chemColor->atomColor.getNum();
426+
for (int ci = 0; ci < 4 && ci < n; ci++) {
427+
fprintf(lg, "ATOMCOLOR %d = %.3f %.3f %.3f\n", ci,
428+
atomColors[ci][0], atomColors[ci][1],
429+
atomColors[ci][2]);
396430
}
431+
fflush(lg);
397432
}
398433
}
399434
const float *atomRadii = chemRadii->atomRadii.getValues(0);
@@ -762,22 +797,24 @@ normalSphereROCA
762797
// in ChemDisplay::GLRender happens before this and cannot show
763798
// what GL_COLOR_MATERIAL has since made of it.
764799
{
765-
const char *where = getenv("ECCE_DEBUG_MATERIAL");
766-
if (where != 0 && theAtom < 4) {
767-
FILE *log = fopen(where, "a");
768-
if (log != 0) {
769-
GLfloat amb[4], emi[4], dif[4], col[4];
770-
glGetMaterialfv(GL_FRONT, GL_AMBIENT, amb);
771-
glGetMaterialfv(GL_FRONT, GL_EMISSION, emi);
772-
glGetMaterialfv(GL_FRONT, GL_DIFFUSE, dif);
773-
glGetFloatv(GL_CURRENT_COLOR, col);
774-
fprintf(log, "SPHERE atom %d colour %.3f %.3f %.3f | "
775-
"amb %.3f %.3f %.3f | emi %.3f %.3f %.3f | "
776-
"dif %.3f %.3f %.3f\n", (int)theAtom,
777-
col[0],col[1],col[2], amb[0],amb[1],amb[2],
778-
emi[0],emi[1],emi[2], dif[0],dif[1],dif[2]);
779-
fclose(log);
780-
}
800+
// #83: was capped to theAtom < 4, which on a 6-atom
801+
// molecule (C,H,H,H,O,H) silently never covered the
802+
// oxygen or the last hydrogen -- exactly the atoms whose
803+
// measured colour showed the tint. Log every atom now
804+
// that the handle is cheap (opened once, not per call).
805+
FILE *log = debugMaterialLog();
806+
if (log != 0) {
807+
GLfloat amb[4], emi[4], dif[4], col[4];
808+
glGetMaterialfv(GL_FRONT, GL_AMBIENT, amb);
809+
glGetMaterialfv(GL_FRONT, GL_EMISSION, emi);
810+
glGetMaterialfv(GL_FRONT, GL_DIFFUSE, dif);
811+
glGetFloatv(GL_CURRENT_COLOR, col);
812+
fprintf(log, "SPHERE atom %d colour %.3f %.3f %.3f | "
813+
"amb %.3f %.3f %.3f | emi %.3f %.3f %.3f | "
814+
"dif %.3f %.3f %.3f\n", (int)theAtom,
815+
col[0],col[1],col[2], amb[0],amb[1],amb[2],
816+
emi[0],emi[1],emi[2], dif[0],dif[1],dif[2]);
817+
fflush(log);
781818
}
782819
}
783820

0 commit comments

Comments
 (0)